Remove internal id from switches, lamps, and coils#408
Conversation
Codecov Report
@@ Coverage Diff @@
## master #408 +/- ##
==========================================
- Coverage 83.34% 83.30% -0.05%
==========================================
Files 128 128
Lines 7081 7075 -6
==========================================
- Hits 5902 5894 -8
- Misses 1179 1181 +2
Continue to review full report at Codecov.
|
freezy
left a comment
freezy
left a comment
There was a problem hiding this comment.
Thanks!
Great to hear that this can be simplified. :)
Will test and look through PinMAME tomorrow.
| foreach (var sw in MainComponent.AvailableSwitches) { | ||
| if (sw.InternalId > 0) { | ||
| _stackSwitches[sw.InternalId - 1] = CreateSwitch(sw.Id, false, MainComponent.Type == TroughType.ModernOpto ? SwitchDefault.NormallyClosed : SwitchDefault.NormallyOpen); | ||
| _switchLookup[sw.Id] = _stackSwitches[sw.InternalId - 1]; | ||
| foreach (var @switch in MainComponent.AvailableSwitches) { | ||
| var match = new Regex(@"^ball_switch_(\d+)$").Match(@switch.Id); | ||
| if (match.Success) { | ||
| int.TryParse(match.Groups[1].Value, out int id); | ||
| if (id > 0) { | ||
| _stackSwitches[id - 1] = CreateSwitch(@switch.Id, false, MainComponent.Type == TroughType.ModernOpto ? SwitchDefault.NormallyClosed : SwitchDefault.NormallyOpen); | ||
| _switchLookup[@switch.Id] = _stackSwitches[id - 1]; | ||
| } |
There was a problem hiding this comment.
This is a bit ugly, but since it's only done once I guess there isn't too much of an impact. I would at least move the regex compilation out of the loop though.
And one thing I would add is a comment when ball_switch_{i} is defined, so it's clear we cannot rename it without changing the regex.
There was a problem hiding this comment.
Done. Just curious, if there are multiple troughs on a playfield, would multiple ball_switch_1s cause any issues?
jsm174
force-pushed
the
misc/remove-internal-id
branch
2 times, most recently
from
942bd72 to
74930ad
Compare
jsm174
force-pushed
the
misc/remove-internal-id
branch
from
74930ad to
3a242c9
Compare
|
Merging this soon. @jsm174 Could you update the MPF repo with these changes as well please? |
|
sure. Will do tonight! |
2 participants
Switch, lamps, and coils in VisualPinball.Engine are identified with an
Idvariable that is a string.PinMAME internally uses integers to identify switches, lamps, and coils.
Because of this, an
InternalIdvariable was added to VisualPinball.Engine to help mapping.When working on RGB lamps for TWD, we had issues and needed to pass
InternalIdto the lamp player.We then noticed missing lamps when auto
Populatinglamps in the lamp manager. Lamps that don't have an internal id default as 0. Autopopulatingoverwrites existing entries by internal ids.Since
InternalIdis real only needed for PinMAME, (Visual Scripting and MPF only useId), we could removeInternalIdall together, and make the PinMAME GLE deal with theId.This PR does exactly that. There are additional PRs for PinMAME, Visual Scripting, and MPF.