freezy / VisualPinball.Engine

:video_game: Visual Pinball Engine for Unity
https://docs.visualpinball.org
GNU General Public License v3.0
398 stars 62 forks source link

Remove internal id from switches, lamps, and coils #408

Closed jsm174 closed 2 years ago

jsm174 commented 2 years ago

Switch, lamps, and coils in VisualPinball.Engine are identified with an Id variable that is a string.

PinMAME internally uses integers to identify switches, lamps, and coils.

Because of this, an InternalId variable was added to VisualPinball.Engine to help mapping.

When working on RGB lamps for TWD, we had issues and needed to pass InternalId to the lamp player.

We then noticed missing lamps when auto Populating lamps in the lamp manager. Lamps that don't have an internal id default as 0. Auto populating overwrites existing entries by internal ids.

Since InternalId is real only needed for PinMAME, (Visual Scripting and MPF only use Id), we could remove InternalId all together, and make the PinMAME GLE deal with the Id.

This PR does exactly that. There are additional PRs for PinMAME, Visual Scripting, and MPF.

codecov[bot] commented 2 years ago

Codecov Report

Merging #408 (dff8097) into master (b663871) will decrease coverage by 0.04%. The diff coverage is 0.00%.

@@            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     
Impacted Files Coverage Δ
...Pinball.Engine/Game/Engines/GamelogicEngineCoil.cs 71.42% <0.00%> (+2.67%) :arrow_up:
...Pinball.Engine/Game/Engines/GamelogicEngineLamp.cs 20.00% <0.00%> (-0.90%) :arrow_down:
...nball.Engine/Game/Engines/GamelogicEngineSwitch.cs 73.33% <0.00%> (-26.67%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 48ddbe1...dff8097. Read the comment docs.

freezy commented 2 years ago

Merging this soon. @jsm174 Could you update the MPF repo with these changes as well please?

jsm174 commented 2 years ago

sure. Will do tonight!