Fix invalid behavior and crash while interacting with whirlpools by heroes on boats

[ihhub]

• update object type after a hero boards a boat and removes the boat sprite from a tile (close #9141)
• fix a crash while a hero tries to disembark from a whirlpool (close #9214)

What was the problem:

1. A hero "owns" tile's object type while stepping on a tile. If a tile was for example marked as bush, then a hero class remembers this type and sets the tile's object type as a hero.
2. In case when a hero board a boat he remembers the type as "boat" and marks as a "hero" object type.
3. In situation when a hero travels by boat and "steps on" an object the tile is going to be marked as "hero" object type. More importantly, if an object was marked as main addon / main object part of that tile it will be moved to bottom layer objects (see Maps::Tiles::setBoat() method). This is the first problem.
4. However, when a hero standing on a tile with an object steps out of a boat the tile is marked as "boat" object type.
5. When a hero boards that boat based on the previous logic the tile is going to be marked as "empty" as no object exists (see void ActionToBoat() function and lines:

   hero.setObjectTypeUnderHero( MP2::OBJ_NONE ); <--- here we explicitly set that no object was under hero
   destinationTile.resetObjectSprite(); <--- here we remove the boat information

   This is the second problem.

6. When a hero steps out of the boat and a player makes a save the tile with a boat is going to be marked as "boat" object type. When the player loads this saves and we actually search for all tiles with Whirlpools that particular tile is not going to be marked as whirlpool since the object type is different. This is the third problem.

The solution:

1. To fix the first problem we introduce Maps::getObjectPartByActionType() function which search for any object part related to a specific object type. So, previously the logic was expecting whirlpool to be set as a main object part / addon but it could be moved by Maps::Tiles::setBoat() method to bottom layer. So, if a tile is marked as Whirlpool we search a whirpool part inside the main object part and within all objects on the bottom layer of the tile.
2. To fix the second problem we update tile's object type before a hero remember it but after we remove the boat:

   destinationTile.resetMainObjectPart(); <--- we remove boat's information from the tile
   destinationTile.updateObjectType(); <--- update object type for this tile if any object previously existed
   hero.setObjectTypeUnderHero( destinationTile.GetObject( true ) ); <--- set the updated object type for hero
   destinationTile.SetObject( MP2::OBJ_HERO ); <-- set the tile's object type as hero

3. To fix the third problem we need to scan every tile on the map for existence of whirlpool object parts even if a tile is not marked as whirlpool. To solve this problem Maps::getObjectParts() function was introduced.

[ihhub]

Hi @zenseii and @oleg-derevenetz , could you please check this pull request whether it fixes the mentioned issue?

[ihhub]

Hi @zenseii and @oleg-derevenetz , could you please check this pull request whether it fixes the mentioned issue?

Please hold on with the review. I found much bigger problem with the code.

[ihhub]

I've tested it on several maps and it works well.

I did notice in the save provided by LiquidYetti that the previously broken whirlpool tile will now contain an ICN object type as unknown. I didn't notice this happening in my fheroes2 map:

19.10.2024 13:07:28: [DBG_DEVEL]        Interface::AdventureMap::mouseCursorAreaPressRight:
******* Tile info *******
Tile index      : 7096, point: (76, 65)
MP2 object type : 167 (Whirlpool)
UID             : 0
ICN object type : 0 (UNKNOWN)
image index     : 255
layer type      : 0 - Object layer
is shadow       : no
region          : 36
ground          : Ocean (isRoad: 0)
ground img index: 13, image flags: 4
passable from   : center, top, top-right, right, bottom-right, bottom, bottom-left, left, top-left
metadata value 1: 0
metadata value 2: 0
metadata value 3: 0
--------- Level 1 --------
UID             : 62
ICN object type : 50 (OBJNWATR.ICN)
image index     : 202
layer type      : 1 - Background layer
is shadow       : no
--- Extra information ---

If this is expected then I'm ready to approve the PR.

Hi @zenseii , this is a leftover from the previous logic. A hero jumped into a boat before making this save file. If it is done with the proposed logic the main object remains whirlpool. However, the engine works even with this case as I added required changes for it.

[Districh-ru]

Hi, @zenseii and @ihhub, it happens because a Summon Boat spell was used by the Purple AI player. And its behavior was not changed in this PR. Technically it is OK - OBJNWATR.ICN object is now in the bottom layer addons, but yes - it looks a little weird that it is not moved back to the main addon. :) We can update this logic in a new PR.

[Branikolog]

Hi, @ihhub An assertion occurred, when I summon a boat standing on a lighthouse and board in it. Assertion.zip

Summon a boat with Astra and step on it.

[Districh-ru]

Hi, @ihhub An assertion occurred, when I summon a boat standing on a lighthouse and board in it.

@ihhub, the AddonsSort() method moves the MP2::OBJ_ICN_TYPE_FLAG32 to the _mainAddon which is probably not good. We may change the behavior of AddonsSort() not to move MP2::OBJ_NONE objects to the _mainAddon by inserting between the 884 and 885 lines the next code:

        if ( getObjectTypeByIcn( highestPriorityAddon._objectIcnType, highestPriorityAddon._imageIndex ) == MP2::OBJ_NONE ) {
            return;
        }

Or we should only don't move only highestPriorityAddon._objectIcnType == MP2::OBJ_ICN_TYPE_FLAG32? What do you think?

[ihhub]

Hi @Districh-ru , I removed object part sorting call for now as I don't think a solution would be that simple.

[ihhub]

Hi @Branikolog , I fixed the assertion. Could you please take a look again?

[ihhub]

Hi @oleg-derevenetz and @Districh-ru , I updated the description of this pull request to explain in details the problems and how they were fixed. I hope now it clarifies many thing. Please let me know if something is not clear.

[sonarcloud[bot]]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud