Banania
Banania copied to clipboard
BenjaminRi
Fix for crash when too many monsters on same spot
The game is crashing when multiple monsters are on the same field because of that.level_array[dst.x][dst.y] is not defined
Hi @klsdfr , thank you for filing this bug and pull request.
I don't quite understand the bug you are describing. Monsters cannot be on the same spot since they always take up the entire square they are in, plus the square they're moving to, while they're moving. I tried reproducing the problem by putting a lot of monsters close to each other, but I could not reproduce it. Could you please describe how to reproduce this issue and on what level? Perhaps a screenshot of the exact situation would help.
This check: typeof that.level_array[dst.x][dst.y].id != 'undefined' should never be necessary. The level_array destination is always defined - if it isn't, that is a bug elsewhere. You can see this in the code, I make sure that each coordinate is initialized with a valid entry and if something moves, the two entries are swapped:
https://github.com/BenjaminRi/Banania/blob/c3f54e3c50af6060c3936f87cec6174f0a858cca/game/game.js#L1251
https://github.com/BenjaminRi/Banania/blob/c3f54e3c50af6060c3936f87cec6174f0a858cca/game/game.js#L1401
Ooh, I see it now. The crash happens when a monster tries to move out of bounds, I believe. Of course, array indices outside the boundaries, like that.level_array[-1][0] are not defined.
But I actually do perform boundary checks, so this bug is subtler, it somehow subverts my boundary checks.
Ok here I reproduced the the error. The game is freezing after the error and has to be reloaded.
As you can see the monsters in the left corner look like they are on the same field.
but nevertheless, I appreciate your work. Played the game a long time ago and now it's a lot of fun again 😅👌
Thank you!
Yeah that's nasty. There's a pretty big bug somewhere inside the move logic. It seems airtight when I read the code, but somewhere in there, illegal moves are allowed.
I think your PR is only a band-aid, the root cause lies deeper.
The bug comes from the is_small property in the walkable check. There is insufficient checking done to prevent multiple monsters from walking into the same spot:
https://github.com/BenjaminRi/Banania/blob/c3f54e3c50af6060c3936f87cec6174f0a858cca/game/game.js#L1311
The issue is fixed with commit a89488d602511ff8a775e6058b78102f3f6d3045 (currently HEAD of master)
There was an issue with the logic that decided where entities (in particular, monsters) can move. However, the walkability check code could use a larger rework (well, the entire code could use a rework), I believe there might still be some subtle issues with pushing blocks simultaneously from multiple directions.
Please let me know if it works for you now. I'm always happy to hear from people who play or modify the game.