missionpinball / mpf-monitor

Graphical utilty which connects to MPF to let you visually control, troubleshoot, and see the status of the machine
http://missionpinball.org
MIT License
11 stars 10 forks source link

Checks variable type before updating #35

Closed atummons closed 9 months ago

atummons commented 2 years ago

This verifies that the variable type matches before updating. Currently it only checks the name, and then updates the value. For instance, if you have a machine variable, but accidently use set instead of set_machine, the variables pane will show the updated value, but its not actually updated because it created a player variable with that name in the actual runtime. This creates a new dictionary to tie the varaible to the type, and then verifies the variable exists of the correct type before updating a row. Otherwise it will enter it. In this case you would have two variables named the same, one of type machine, and one of type player.

atummons commented 2 years ago

If this is an acceptable fix, please let me know. It appears there isn't a test for the variables section. So if this is good, I will write a test case for variables that includes general stuff and this specific change to include.

I didn't make any changes, and one of the tests failed. I can look into that as well, but not sure about the already_hidden check.

atummons commented 2 years ago

It looks like the failure point was removed in logic back in 2020. The setting hidden is now not part of the add_event_to_model, but only update_events. Should we update the test then to not fail for that?

Is there a similar thing to advancetimeandrun()? In mpfmon it looks like the tick event should trigger the event piece. But since no time elapses, it doesn't get called in time.

atummons commented 1 year ago

@toomanybrians Do you have any thoughts on this? I think the fix is good, but it's erroring from prior changes. We can update the tests, based on the previous changes to pass. But wanted someone's opinion before writing my tests and fixing the currently broken tests.

toomanybrians commented 9 months ago

I'm updating MPF Monitor to 0.57 and taking a look at this. Good find here! Your logic makes sense. I'm going to implement it slightly differently, instead of a separate dict() to track types, I'm going to change the keys in the variables dict to be tuples of (variable, type).

Thanks for finding this!