Open avanwinkle opened 3 years ago
Do you have an example which triggers this in LS? I would like to add an unit or integration test for that. Any way to reproduce this would help.
Your fix makes sense in general. Guess this is related to usage of "_overwrite". We could just add your fix but it might break again in the future. To prevent this I would like a failing test first which is then fixed by your change.
Here's a block from my code that triggers the crash:
event_player:
timer_reaper_charging_complete: fire_reapercannon
timer_reaper_firing_complete:
- flippers_off
- restart_reaper|4s
restart_reaper:
- flippers_on
- enable_random_reapershot
player_reaper_hp{player_num==current_player.number}:
- humanreaper_failed{value>0 and current_player.reaper_rounds<4}
- humanreaper_complete{value<=0 or current_player.reaper_rounds>=4}
ball_save_reaper_squadmate_save_saving_ball:
kill_squadmate:
squadmate: random
Specifically, it's the final event with the dict-type declaration that causes the error. If I change that to be just a simple string or list, the error goes away.
I suppose if I were to convert that last item to be a list of dicts, it would also pass. So maybe the fix would be to coerce the children of event_player to be a list, even if they are a dict? Converting strings to list works well enough.
And maybe I should have just had it like this the whole time?
ball_save_reaper_squadmate_save_saving_ball:
- kill_squadmate:
squadmate: random
I tried to reproduce your issue today but the example seems to work for me. As an example for mpf test
:
event_player:
timer_reaper_charging_complete: fire_reapercannon
timer_reaper_firing_complete:
- flippers_off
- restart_reaper|4s
restart_reaper:
- flippers_on
- enable_random_reapershot
player_reaper_hp{player_num==current_player.number}:
- humanreaper_failed{value>0 and current_player.reaper_rounds<4}
- humanreaper_complete{value<=0 or current_player.reaper_rounds>=4}
ball_save_reaper_squadmate_save_saving_ball:
kill_squadmate:
squadmate: random
##! test
#! start_game
#! post ball_save_reaper_squadmate_save_saving_ball
This passes in dev. Also if I add it to a machine this seems to work fine in mpf-ls. Did you fix this already? Or did I miss something?
I knew I'd seen this before! I threw a quick fix into this PR (https://github.com/missionpinball/mpf/pull/1541#pullrequestreview-531082268) before seeing that it broke all the tests, so I reverted it.
I do still see the MPF-LS error when I open this file in VSCode, though. It doesn't hurt anything MPF (that I can find or remember), but the language server isn't happy. Have the latest dev pulls of both MPF and LS.
I will have another look
There's a bug in
Util.dict_merge()
that I uncovered when using the MPF Language Server. The merge method attempts to access a key0
of the dict, which is often not found, and the file validation fails.The code block in question is the following iteration that merges two dicts,
a
andb
:From digging into the events of mine that trigger this, the culprit seems to be event players where the events are a combination of strings, lists, and dicts. I'm having trouble teasing out exactly how the dict merge is working, but I think it might be as simple as a typo on the failing line.
All that logic is about
v
being a list, but the line saysif isinstance(v, dict) and v[0]
. When I change that line to readif isinstance(v, list) and v[0]
the error goes away and the language server works properly.I'm assuming that this doesn't show in normal playback because the callers of the merge are wrapping the error, but it could also be that I'm misreading the code and my change is actually breaking functionality. Merely changing the instance type from
dict
tolist
causes a million test failures because of dicts with empty lists. Previously those empty lists failed thedict
type so thev[0]
was never evaulated, but callingv[0]
on an empty list will throw. So I had to make the final line look like this:With that, all of the tests pass again.
So... am I on the right track here? Or am I overlooking something?