save current tile map before loop in case tile map changed in overlap handlers

[felixtsu]

microsoft/pxt-arcade#4819

[jwunderl]

hmm, this one's pretty subtle so I have to think about it a little extra-carefully -- if this is merged, then it is possible that you can run an overlap event for a tile that is no longer part of the map -- e.g. if one teleports you to a shop and you happen to also have hit water that frame it could say "I'm drowning" and have logic to teleport you back to the shore which is now the middle of a display or something.

I think where I'd lean is that instead of continuing to fire events, drop any further overlaps for the frame after a tile overlap event that changes the tilemap or the position of the sprite -- at that point we can't really make any promises anymore without re-checking everything and making the code way more messy / slower so I think it's better to just drop it and wait for the next frame when we can get back to well defined behavior. Does that sound reasonable to you / tagging in @riknoll for a consensus here

[felixtsu]

hmmmm, on second thought it‘s really quite subtle. I tried to consider these scenarios to my students as engine users:

1. two adjacent tiles teleporting to different tile maps, sprite move to center of the two tiles, randomly teleported to one of the tile maps
   my students: Check tile map, ah, I placed these portals too close that the sprite stepped on both of them, teleported randomly, make sense, I should change my tile map design.

2. two adjacent tiles teleporting to different tile maps, sprite move to center of the two tiles, teleported to the same tile map connected by the left tile every time
   my students: Hmmmmm, the engine decided that the sprite stepped on the left tile first and ran my code for that tile during which the sprite was teleported to another tile map, the tile on the right was considered not stepped on, make sense.

3.a two adjacent tiles, left one is a portal, right one is a trap ( -10 life) , sprite move to center of the two tiles, teleported to new tile map with no harm
3.b two adjacent tiles, left one is a trap ( -10 life), right one is a portal, , sprite move to center of the two tiles, teleported to new tile map, life - 10
my students: Why??????

[riknoll]

this is a tough one, and i'm not sure there is a reasonable behavior that will work for every game. that being said, i think this is the most important rule uphold to match the expectations of users:

If a tile overlap event fires at a given location, when i check the tile at that location inside of the event handler then it should always have the tile specified in the event block (assuming i didn't change it earlier in this event block)

beyond that, i don't think the behavior is super important and will only make a difference in some real edge cases. i don't think that delaying the event by a frame makes the end result easier to understand, so i would lean towards firing more events instead of potentially dropping some by halting events when the map changes. as long as the tile is the correct one, fire away!

[jwunderl]

Fair enough -- so this would mean leaving the current overlap behavior as is (use current tilemap when running overlap handler, even if it has changed in a previous handler).

I'd still push for bailing out of overlap checks when the position of the sprite changed in a previous handler, though -- this case shows a very weird behavior, where changing the tilemap + changing location can make it so an overlap event runs when a sprite is no longer touching the other tile and technically never was on top of a tile of that type-- which would change your description slightly to

If a tile overlap event fires at a given location, when i check the tile at that location inside of the event handler then it should always have the tile specified in the event block and the sprite should be on top of that location (assuming i didn't change it earlier in this event block)

[felixtsu]

Sorry for the late reply.

After discussion with my student who reported the original bug, I realized that I made a mistake in the reproducing project and the fix in PR is invalid.

To reproduce the issue more precisely, in this project
https://makecode.com/_dm76Y28iMfHx

Tilemap 1

Tilemap 2

Tile(2,5) in tilemap 2 is a wall that the sprite should not overlap, but stepping on tile(1,5) in tilemap 1 will change current tilemap to tilemap2, and bypasses the isObstacle() check

I think this is a corner case but very weird behavior thoug