Genkido: Wrong collision for event with tile graphic

[florianessl]

Name of the game: Genkido

Attach files (as a .zip archive or link them)

Download for game & savefile is found here:
https://community.easyrpg.org/t/gamebreaking-collision-issue-in-genkidou-short-platformer-from-2006/1607

Describe the issue in detail and how to reproduce it:

Access the platforms on the right side by avoiding the enemies and touch both barrels to create a passable way on the left platform. (First, drop the lower one of the barrels, then go up the ladder and sacrifice some HP by touching the pig enemy. Then you have access to the other barrel and can make your way back)
In RPG_RT this new passageway can be used to walk to the exit. In EasyRPG this remains inaccessible. The player can't walk over the barrel.

Notes:

In this specific example, the barrel event itself is based on a non-passable tile (EV0022), but on the spot where it lands, another event (EV0023) is activated which would take precedence for collision detection.
Tested across a few different RPG_RT versions, including 2k3 & the newest MP build. All seem to interpret this the same way.

Also: Moving EV0023 to a lower ID makes the tile impossible to pass on any version. Event ID takes precedence here.

For some reason EasyRPG Player has another special condition in its collision detection:
When checking for passability by examining all events with "below" status on the target spot X/Y, it only considers events when the tile_id is greater than zero. This filters out EV0023 & makes walking over the this spot impossible.

Patching this should be quite simple. But one has to wonder, why this specific check is there & if this was introduced to fix some other rare issue?

 src/game_map.cpp | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/src/game_map.cpp b/src/game_map.cpp
index 5180fdd43..e47162820 100644
--- a/src/game_map.cpp
+++ b/src/game_map.cpp
@@ -1018,7 +1018,7 @@ bool Game_Map::IsPassableTile(
 			}
 		}
 
-		// Highest ID event with layer=below, not through, and a tile graphic wins.
+		// Highest ID event with layer=below and not 'through' wins.
 		int event_tile_id = 0;
 		for (auto& ev: events) {
 			if (self == &ev) {
@@ -1028,10 +1028,7 @@ bool Game_Map::IsPassableTile(
 				continue;
 			}
 			if (ev.IsInPosition(x, y) && ev.GetLayer() == lcf::rpg::EventPage::Layers_below) {
-				int tile_id = ev.GetTileId();
-				if (tile_id > 0) {
-					event_tile_id = tile_id;
-				}
+				event_tile_id = ev.GetTileId();
 			}
 		}

[florianessl]

Ok, I looked up the initial issue that was used as a base for the Player's "MakeWay" implementation. For reference: #1675
With this impressive test data, I wonder how anything slipped trough 😅

[Ghabry]

This tile Id check will also detect whether it is using a charset graphic.

So maybe the bug is that we handle a tile with id 0 the same as a event graphic. 🤔

Needs some testing.

Funny edge case that only happens when two events overlap 😅🙈

[Ghabry]

Your patch breaks when I set the tile to a character because they also have Tile ID 0 but I think you are on the correct track.

---

Did some more tests:

I hacked Tile 0 to be not passable and then created this layout (the upper layer map tiles are passable):

Result: You can walk on it. ❗

Then with a non-passable event tile.

Result: You cannot walk on it.

Conclusion: Event Tile ID is not considered for passability checks. This is the check if (event_tile_id > 0).

---

(Tile 0 was changed to passable again)

Additionally as you correctly stated (and what the code already does sans the ID 0 edge case): The highest event ID shadows lower event IDs and event_tile_id = 0 does not matter here, it always picks the first event that has a tile graphic.

Proof:

ID order: Carol > Empty Tile > Barrel Tile

A move route positions all of them on the Empty Tile.

Result: You can walk on it.

Conlusion: The chara event (Carol) is ignored even though it's the highest. The empty tile is picked and shadows the barrel.

---

Proposed patch:

diff --git a/src/game_map.cpp b/src/game_map.cpp
index 5180fdd43..8334d2010 100644
--- a/src/game_map.cpp
+++ b/src/game_map.cpp
@@ -1028,9 +1028,8 @@ bool Game_Map::IsPassableTile(
 				continue;
 			}
 			if (ev.IsInPosition(x, y) && ev.GetLayer() == lcf::rpg::EventPage::Layers_below) {
-				int tile_id = ev.GetTileId();
-				if (tile_id > 0) {
-					event_tile_id = tile_id;
+				if (ev.HasTileSprite()) {
+					event_tile_id = ev.GetTileId();
 				}
 			}
 		}

This also doesn't invalidate the comment because it is still and a tile graphic wins.

That check is also harmless because later the code checks if (event_tile_id > 0 so it has no side effects besides shadowing the event with the lower ID.

[Ghabry]

Fixing this for 0.8.1 is probably too risky and this is such a huge edge case that only one game was discovered after such a long time.

Lets move this to 0.8.2 so we don't regress anything right before the release xD

[Ghabry]

Lets move this to 0.8.2 so we don't regress anything right before the release xD

cough that aged badly cough. But this could be added to 0.8.1.1 so when we regress again we can just revert it for 0.8.1.2 ;)