Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Additionally map string-parsed IDs when mapping coils #430

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

freezy
Copy link
Owner

@freezy freezy commented Aug 20, 2022

There was a regression introduced when we cleared internal (int) IDs in the GLE in favor of purely string-based IDs.

The coil player currently blindly looks up the ID it gets from the GLE, which in the case of PinMAME, is a stringified integer. However, we often prefix coil IDs with "0" to get proper sorting, and it's also how they are named in the manual. So a "04" coil mapping won't match a "4" event from the GLE.

This patch additionally maps coils to their stringified integer, if it's not the same as the original name.

There will be another problem, the other way around, for switches. I will update this PR with a fix for that too, when I can reproduce.

I'll also do the same for lamps. @jsm174 let me know your thoughts already.

@freezy freezy requested a review from jsm174 August 20, 2022 23:05
@freezy freezy self-assigned this Aug 20, 2022
@freezy
Copy link
Owner Author

freezy commented Aug 20, 2022

Also need to check whether that doesn't screw up the coil statuses, because CoilStatuses is not distinct anymore.

@jsm174
Copy link
Collaborator

jsm174 commented Aug 21, 2022

However, we often prefix coil IDs with "0" to get proper sorting

The treeview control used for switch and coil managers was update to support sorting by number here:

58c77cf

@freezy
Copy link
Owner Author

freezy commented Aug 21, 2022

Ah, cool. Still want to support people prefixing names with "0" though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants