fix: enable transition to ESCRoom even if esc_script is not set #502
Conversation
When creating a new `ESCRoom`, there was a bug where it was not possible to transition into the room if the room's `esc_script` was not set. Apparently, `_perform_script_events()` would bail out early when `room.esc_script` is falsy, which seems to be why the transition does not happen. Perhaps this defensive check was necessary at one time, but that does not appear to be the case today because the only code that reads `room.esc_script` in this file is within the `_run_script_event()` function, which does its own defensive check for `room.esc_script`. As such, it appears that we can let the rest of `_perform_script_events()` do its thing even when `room.esc_script` is falsy (i.e., `""`).
bolinfest
requested review from
StraToN,
BHSDuncan and
dploeger
as code owners
bolinfest
changed the title
|
This may be related to godot-escoria/escoria-issues#95 (fixed) |
|
This check was added by @BHSDuncan in 99dc1e0 (#485). Maybe he can give us some details on the reason why the check was re-introduced? |
I didn't actually reintroduce it: I factored it to a spot that (at the time) made more sense since where the places it was originally in could be reduced. The ticket that commit was a part involved introducing no new functionality, and if code was moved around, it would've been for the purpose of maintaining the same functionality (no more, no less) but with what was supposed to be cleaner/more efficient code. TL;DR: It wasn't really "re-introduced" per se; it was just factored, so if the check can safely be removed, then by all means. |
|
We’re there any outstanding concerns with this one? |
|
Thanks a lot ! |
When creating a new
ESCRoom, there was a bug where it was not possibleto transition into the room if the room's
esc_scriptwas not set.Apparently,
_perform_script_events()would bail out early whenroom.esc_scriptis falsy, which seems to be why the transition doesnot happen.
Perhaps this defensive check was necessary at one time, but that does
not appear to be the case today because the only code that reads
room.esc_scriptin this file is within the_run_script_event()function, which does its own defensive check for
room.esc_script. Assuch, it appears that we can let the rest of
_perform_script_events()do its thing even when
room.esc_scriptis falsy (i.e.,"").