From db72a64d3f55c038a72995d44adb90c5c43ab957 Mon Sep 17 00:00:00 2001 From: Ihar Hubchyk Date: Sat, 24 Feb 2024 23:52:23 +0800 Subject: [PATCH 01/10] Fix invalid calculation of "short" object passability relates to #4495 --- src/fheroes2/maps/maps_tiles.cpp | 76 +++++++++++++++----------------- 1 file changed, 36 insertions(+), 40 deletions(-) diff --git a/src/fheroes2/maps/maps_tiles.cpp b/src/fheroes2/maps/maps_tiles.cpp index abfdb9206e8..aa8abf31357 100644 --- a/src/fheroes2/maps/maps_tiles.cpp +++ b/src/fheroes2/maps/maps_tiles.cpp @@ -28,6 +28,7 @@ #include #include #include +#include #include #include #include @@ -248,46 +249,39 @@ namespace } #endif - bool isShortObject( const MP2::MapObjectType objectType ) + bool isShortObject( const uint32_t uid, const int32_t startIndex ) { - // Some objects allow middle moves even being attached to the bottom. - // These object actually don't have any sprites on tiles above them within addon 2 level objects. - // TODO: find a better way to do not hardcode values here. + const int32_t startY = startIndex / world.w(); - switch ( objectType ) { - case MP2::OBJ_HALFLING_HOLE: - case MP2::OBJ_NON_ACTION_HALFLING_HOLE: - case MP2::OBJ_LEAN_TO: - case MP2::OBJ_WATER_LAKE: - case MP2::OBJ_TAR_PIT: - case MP2::OBJ_MERCENARY_CAMP: - case MP2::OBJ_NON_ACTION_MERCENARY_CAMP: - case MP2::OBJ_STANDING_STONES: - case MP2::OBJ_SHRINE_FIRST_CIRCLE: - case MP2::OBJ_SHRINE_SECOND_CIRCLE: - case MP2::OBJ_SHRINE_THIRD_CIRCLE: - case MP2::OBJ_MAGIC_GARDEN: - case MP2::OBJ_RUINS: - case MP2::OBJ_NON_ACTION_RUINS: - case MP2::OBJ_SIGN: - case MP2::OBJ_IDOL: - case MP2::OBJ_STONE_LITHS: - case MP2::OBJ_NON_ACTION_STONE_LITHS: - case MP2::OBJ_WAGON: - case MP2::OBJ_WAGON_CAMP: - case MP2::OBJ_NON_ACTION_WAGON_CAMP: - case MP2::OBJ_GOBLIN_HUT: - case MP2::OBJ_FAERIE_RING: - case MP2::OBJ_NON_ACTION_FAERIE_RING: - case MP2::OBJ_BARRIER: - case MP2::OBJ_MAGIC_WELL: - case MP2::OBJ_NOTHING_SPECIAL: - return true; - default: - break; + std::deque indexToTraverse{ startIndex }; + std::set traversedIndex; + + const Directions directions = Direction::All(); + + while ( !indexToTraverse.empty() ) { + traversedIndex.emplace( indexToTraverse.front() ); + + for ( const int direction : directions ) { + if ( !Maps::isValidDirection( indexToTraverse.front(), direction ) ) { + continue; + } + + const int32_t newIndex = Maps::GetDirectionIndex( indexToTraverse.front(), direction ); + const Maps::Tiles & tile = world.GetTiles( newIndex ); + + if ( Maps::doesTileHaveObjectUID( tile, uid ) && traversedIndex.count( newIndex ) == 0 ) { + if ( ( newIndex / world.w() ) != startY ) { + return false; + } + + indexToTraverse.emplace_back( newIndex ); + } + } + + indexToTraverse.pop_front(); } - return false; + return true; } bool isDetachedObjectType( const MP2::MapObjectType objectType ) @@ -507,7 +501,7 @@ void Maps::Tiles::Init( int32_t index, const MP2::mp2tile_t & mp2 ) _isTileMarkedAsRoad = true; } - if ( mp2.mapObjectType == MP2::OBJ_NONE && ( layerType == Maps::ObjectLayerType::SHADOW_LAYER || layerType == Maps::ObjectLayerType::TERRAIN_LAYER ) ) { + if ( _mainObjectType == MP2::OBJ_NONE && ( layerType == Maps::ObjectLayerType::SHADOW_LAYER || layerType == Maps::ObjectLayerType::TERRAIN_LAYER ) ) { // If an object sits on shadow or terrain layer then we should put it as a bottom layer add-on. if ( bottomObjectIcnType != MP2::ObjectIcnType::OBJ_ICN_TYPE_UNKNOWN ) { _addonBottomLayer.emplace_back( layerType, mp2.level1ObjectUID, bottomObjectIcnType, mp2.bottomIcnImageIndex ); @@ -790,9 +784,11 @@ void Maps::Tiles::updatePassability() const MP2::MapObjectType bottomTileObjectType = bottomTile.GetObject( false ); const MP2::MapObjectType correctedObjectType = MP2::getBaseActionObjectType( bottomTileObjectType ); + const bool isBottomObjectShort = isShortObject( bottomTile.GetObjectUID(), bottomTile.GetIndex() ); + if ( MP2::isActionObject( bottomTileObjectType ) ) { if ( ( MP2::getActionObjectDirection( bottomTileObjectType ) & Direction::TOP ) == 0 ) { - if ( isShortObject( bottomTileObjectType ) ) { + if ( isBottomObjectShort ) { _tilePassabilityDirections &= ~Direction::BOTTOM; } else { @@ -802,10 +798,10 @@ void Maps::Tiles::updatePassability() } } else if ( bottomTile._mainObjectType != MP2::OBJ_NONE && correctedObjectType != bottomTileObjectType && MP2::isActionObject( correctedObjectType ) - && isShortObject( correctedObjectType ) && ( bottomTile.getOriginalPassability() & Direction::TOP ) == 0 ) { + && isBottomObjectShort && ( bottomTile.getOriginalPassability() & Direction::TOP ) == 0 ) { _tilePassabilityDirections &= ~Direction::BOTTOM; } - else if ( isShortObject( bottomTileObjectType ) + else if ( isBottomObjectShort || ( !bottomTile.containsAnyObjectIcnType( getValidObjectIcnTypes() ) && ( isCombinedObject( objectType ) || isCombinedObject( bottomTileObjectType ) ) ) ) { _tilePassabilityDirections &= ~Direction::BOTTOM; From a846ec0425f07b618390cbdc216a048c38fe486e Mon Sep 17 00:00:00 2001 From: Ihar Hubchyk Date: Sat, 24 Feb 2024 23:53:03 +0800 Subject: [PATCH 02/10] Add missing change --- src/fheroes2/maps/maps_tiles_helper.cpp | 21 +++++++++++++++++++++ src/fheroes2/maps/maps_tiles_helper.h | 2 ++ 2 files changed, 23 insertions(+) diff --git a/src/fheroes2/maps/maps_tiles_helper.cpp b/src/fheroes2/maps/maps_tiles_helper.cpp index e0126a652d0..47a5995577e 100644 --- a/src/fheroes2/maps/maps_tiles_helper.cpp +++ b/src/fheroes2/maps/maps_tiles_helper.cpp @@ -3120,6 +3120,27 @@ namespace Maps } } + bool doesTileHaveObjectUID( const Tiles & tile, const uint32_t uid ) + { + if ( tile.GetObjectUID() == uid ) { + return true; + } + + for ( const TilesAddon & addon : tile.getBottomLayerAddons() ) { + if ( addon._uid == uid ) { + return true; + } + } + + for ( const TilesAddon & addon : tile.getTopLayerAddons() ) { + if ( addon._uid == uid ) { + return true; + } + } + + return false; + } + bool removeObjectTypeFromTile( Tiles & tile, const MP2::ObjectIcnType objectIcnType ) { if ( tile.getObjectIdByObjectIcnType( objectIcnType ) == 0 ) { diff --git a/src/fheroes2/maps/maps_tiles_helper.h b/src/fheroes2/maps/maps_tiles_helper.h index 02920426dc1..3c73b1fb2e4 100644 --- a/src/fheroes2/maps/maps_tiles_helper.h +++ b/src/fheroes2/maps/maps_tiles_helper.h @@ -176,6 +176,8 @@ namespace Maps // Determine the fog direction in the area between min and max positions for given player(s) color code and store it in corresponding tile data. void updateFogDirectionsInArea( const fheroes2::Point & minPos, const fheroes2::Point & maxPos, const int32_t color ); + bool doesTileHaveObjectUID( const Tiles & tile, const uint32_t uid ); + // The functions below are used only in the map Editor. void setTerrainOnTiles( const int32_t startTileId, const int32_t endTileId, const int groundId ); From ddbb1fc2704116747252c991f0be59ee6f2b6a5f Mon Sep 17 00:00:00 2001 From: Ihar Hubchyk Date: Sat, 24 Feb 2024 23:54:00 +0800 Subject: [PATCH 03/10] Use reference --- src/fheroes2/maps/maps_tiles.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/fheroes2/maps/maps_tiles.cpp b/src/fheroes2/maps/maps_tiles.cpp index aa8abf31357..c2a4dab78a0 100644 --- a/src/fheroes2/maps/maps_tiles.cpp +++ b/src/fheroes2/maps/maps_tiles.cpp @@ -256,7 +256,7 @@ namespace std::deque indexToTraverse{ startIndex }; std::set traversedIndex; - const Directions directions = Direction::All(); + const Directions & directions = Direction::All(); while ( !indexToTraverse.empty() ) { traversedIndex.emplace( indexToTraverse.front() ); From 0f29460c5cb3b45891a9af185ee9d727d6dd194d Mon Sep 17 00:00:00 2001 From: Ihar Hubchyk Date: Sat, 24 Feb 2024 23:56:35 +0800 Subject: [PATCH 04/10] Add a comment --- src/fheroes2/maps/maps_tiles.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/fheroes2/maps/maps_tiles.cpp b/src/fheroes2/maps/maps_tiles.cpp index c2a4dab78a0..c33866a6d5c 100644 --- a/src/fheroes2/maps/maps_tiles.cpp +++ b/src/fheroes2/maps/maps_tiles.cpp @@ -249,6 +249,7 @@ namespace } #endif + // An object is considered as short if its height is no more than 1 tile. bool isShortObject( const uint32_t uid, const int32_t startIndex ) { const int32_t startY = startIndex / world.w(); From aeca8699d1eebbd44e31482b94c7626ea26cd13e Mon Sep 17 00:00:00 2001 From: Ihar Hubchyk Date: Sun, 25 Feb 2024 00:06:13 +0800 Subject: [PATCH 05/10] Move function to the correct place --- src/fheroes2/maps/maps_tiles.cpp | 38 +------------------------ src/fheroes2/maps/maps_tiles_helper.cpp | 38 +++++++++++++++++++++++++ src/fheroes2/maps/maps_tiles_helper.h | 3 ++ 3 files changed, 42 insertions(+), 37 deletions(-) diff --git a/src/fheroes2/maps/maps_tiles.cpp b/src/fheroes2/maps/maps_tiles.cpp index c33866a6d5c..eab62c1a387 100644 --- a/src/fheroes2/maps/maps_tiles.cpp +++ b/src/fheroes2/maps/maps_tiles.cpp @@ -249,42 +249,6 @@ namespace } #endif - // An object is considered as short if its height is no more than 1 tile. - bool isShortObject( const uint32_t uid, const int32_t startIndex ) - { - const int32_t startY = startIndex / world.w(); - - std::deque indexToTraverse{ startIndex }; - std::set traversedIndex; - - const Directions & directions = Direction::All(); - - while ( !indexToTraverse.empty() ) { - traversedIndex.emplace( indexToTraverse.front() ); - - for ( const int direction : directions ) { - if ( !Maps::isValidDirection( indexToTraverse.front(), direction ) ) { - continue; - } - - const int32_t newIndex = Maps::GetDirectionIndex( indexToTraverse.front(), direction ); - const Maps::Tiles & tile = world.GetTiles( newIndex ); - - if ( Maps::doesTileHaveObjectUID( tile, uid ) && traversedIndex.count( newIndex ) == 0 ) { - if ( ( newIndex / world.w() ) != startY ) { - return false; - } - - indexToTraverse.emplace_back( newIndex ); - } - } - - indexToTraverse.pop_front(); - } - - return true; - } - bool isDetachedObjectType( const MP2::MapObjectType objectType ) { // Some objects do not take into account other objects below them. @@ -785,7 +749,7 @@ void Maps::Tiles::updatePassability() const MP2::MapObjectType bottomTileObjectType = bottomTile.GetObject( false ); const MP2::MapObjectType correctedObjectType = MP2::getBaseActionObjectType( bottomTileObjectType ); - const bool isBottomObjectShort = isShortObject( bottomTile.GetObjectUID(), bottomTile.GetIndex() ); + const bool isBottomObjectShort = isMainObjectShort( bottomTile ); if ( MP2::isActionObject( bottomTileObjectType ) ) { if ( ( MP2::getActionObjectDirection( bottomTileObjectType ) & Direction::TOP ) == 0 ) { diff --git a/src/fheroes2/maps/maps_tiles_helper.cpp b/src/fheroes2/maps/maps_tiles_helper.cpp index 47a5995577e..27045aa7716 100644 --- a/src/fheroes2/maps/maps_tiles_helper.cpp +++ b/src/fheroes2/maps/maps_tiles_helper.cpp @@ -3141,6 +3141,44 @@ namespace Maps return false; } + bool isMainObjectShort( const Tiles & tile ) + { + const uint32_t uid = tile.GetObjectUID(); + const int32_t startIndex = tile.GetIndex(); + + const int32_t startY = startIndex / world.w(); + + std::deque indexToTraverse{ startIndex }; + std::set traversedIndex; + + const Directions & directions = Direction::All(); + + while ( !indexToTraverse.empty() ) { + traversedIndex.emplace( indexToTraverse.front() ); + + for ( const int direction : directions ) { + if ( !Maps::isValidDirection( indexToTraverse.front(), direction ) ) { + continue; + } + + const int32_t newIndex = Maps::GetDirectionIndex( indexToTraverse.front(), direction ); + const Maps::Tiles & newTile = world.GetTiles( newIndex ); + + if ( Maps::doesTileHaveObjectUID( newTile, uid ) && traversedIndex.count( newIndex ) == 0 ) { + if ( ( newIndex / world.w() ) != startY ) { + return false; + } + + indexToTraverse.emplace_back( newIndex ); + } + } + + indexToTraverse.pop_front(); + } + + return true; + } + bool removeObjectTypeFromTile( Tiles & tile, const MP2::ObjectIcnType objectIcnType ) { if ( tile.getObjectIdByObjectIcnType( objectIcnType ) == 0 ) { diff --git a/src/fheroes2/maps/maps_tiles_helper.h b/src/fheroes2/maps/maps_tiles_helper.h index 3c73b1fb2e4..9ac5b27f51c 100644 --- a/src/fheroes2/maps/maps_tiles_helper.h +++ b/src/fheroes2/maps/maps_tiles_helper.h @@ -178,6 +178,9 @@ namespace Maps bool doesTileHaveObjectUID( const Tiles & tile, const uint32_t uid ); + // An object is considered as short if its height is no more than 1 tile. + bool isMainObjectShort( const Tiles & tile ); + // The functions below are used only in the map Editor. void setTerrainOnTiles( const int32_t startTileId, const int32_t endTileId, const int groundId ); From 2e43f59f2db3ec8bf010ae28d291120fdc41127b Mon Sep 17 00:00:00 2001 From: Ihar Hubchyk Date: Sun, 25 Feb 2024 00:08:10 +0800 Subject: [PATCH 06/10] Make IWYU happy --- src/fheroes2/maps/maps_tiles.cpp | 1 - src/fheroes2/maps/maps_tiles_helper.cpp | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/src/fheroes2/maps/maps_tiles.cpp b/src/fheroes2/maps/maps_tiles.cpp index eab62c1a387..d25f544f04d 100644 --- a/src/fheroes2/maps/maps_tiles.cpp +++ b/src/fheroes2/maps/maps_tiles.cpp @@ -28,7 +28,6 @@ #include #include #include -#include #include #include #include diff --git a/src/fheroes2/maps/maps_tiles_helper.cpp b/src/fheroes2/maps/maps_tiles_helper.cpp index 27045aa7716..3659b63049b 100644 --- a/src/fheroes2/maps/maps_tiles_helper.cpp +++ b/src/fheroes2/maps/maps_tiles_helper.cpp @@ -25,6 +25,7 @@ #include #include #include +#include #include #include #include From 4f8c68d8291d21e31aeb938cce7566f2f3b3ab5e Mon Sep 17 00:00:00 2001 From: Ihar Hubchyk Date: Sun, 25 Feb 2024 00:09:15 +0800 Subject: [PATCH 07/10] Remove extra namespace usage --- src/fheroes2/maps/maps_tiles_helper.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/fheroes2/maps/maps_tiles_helper.cpp b/src/fheroes2/maps/maps_tiles_helper.cpp index 3659b63049b..c20185e9dd7 100644 --- a/src/fheroes2/maps/maps_tiles_helper.cpp +++ b/src/fheroes2/maps/maps_tiles_helper.cpp @@ -3158,14 +3158,14 @@ namespace Maps traversedIndex.emplace( indexToTraverse.front() ); for ( const int direction : directions ) { - if ( !Maps::isValidDirection( indexToTraverse.front(), direction ) ) { + if ( !isValidDirection( indexToTraverse.front(), direction ) ) { continue; } const int32_t newIndex = Maps::GetDirectionIndex( indexToTraverse.front(), direction ); - const Maps::Tiles & newTile = world.GetTiles( newIndex ); + const Tiles & newTile = world.GetTiles( newIndex ); - if ( Maps::doesTileHaveObjectUID( newTile, uid ) && traversedIndex.count( newIndex ) == 0 ) { + if ( doesTileHaveObjectUID( newTile, uid ) && traversedIndex.count( newIndex ) == 0 ) { if ( ( newIndex / world.w() ) != startY ) { return false; } From a8feb2f721355ebf53b818cf898f4374eadbbd61 Mon Sep 17 00:00:00 2001 From: Ihar Hubchyk Date: Sun, 25 Feb 2024 17:41:07 +0800 Subject: [PATCH 08/10] Update the code --- src/fheroes2/maps/maps_tiles.cpp | 21 +++++++++++---------- src/fheroes2/maps/maps_tiles.h | 13 +++++++++---- 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/src/fheroes2/maps/maps_tiles.cpp b/src/fheroes2/maps/maps_tiles.cpp index d25f544f04d..68f225462b8 100644 --- a/src/fheroes2/maps/maps_tiles.cpp +++ b/src/fheroes2/maps/maps_tiles.cpp @@ -651,6 +651,17 @@ int Maps::Tiles::getBoatDirection() const int Maps::Tiles::getOriginalPassability() const { + // Run through all objects in this tile and create the passability based on all of them. + if ( isValidReefsSprite( _mainAddon._objectIcnType, _mainAddon._imageIndex ) ) { + return 0; + } + + for ( const TilesAddon & addon : _addonBottomLayer ) { + if ( isValidReefsSprite( addon._objectIcnType, addon._imageIndex ) ) { + return 0; + } + } + const MP2::MapObjectType objectType = GetObject( false ); if ( MP2::isActionObject( objectType ) ) { @@ -662,16 +673,6 @@ int Maps::Tiles::getOriginalPassability() const return DIRECTION_ALL; } - if ( isValidReefsSprite( _mainAddon._objectIcnType, _mainAddon._imageIndex ) ) { - return 0; - } - - for ( const TilesAddon & addon : _addonBottomLayer ) { - if ( isValidReefsSprite( addon._objectIcnType, addon._imageIndex ) ) { - return 0; - } - } - // Objects have fixed passability. return DIRECTION_CENTER_ROW | DIRECTION_BOTTOM_ROW; } diff --git a/src/fheroes2/maps/maps_tiles.h b/src/fheroes2/maps/maps_tiles.h index dd8478a37aa..5b9833ba69d 100644 --- a/src/fheroes2/maps/maps_tiles.h +++ b/src/fheroes2/maps/maps_tiles.h @@ -42,12 +42,17 @@ class StreamBase; namespace Maps { + // Layer types. They affect passability and also rendering. Rendering must be in the following order: + // - terrain objects (they have no shadows) + // - shadows + // - background objects + // - objects enum ObjectLayerType : uint8_t { - OBJECT_LAYER = 0, // main and action objects like mines, forest, mountains, castles and etc. - BACKGROUND_LAYER = 1, // background objects like lakes or bushes. - SHADOW_LAYER = 2, // shadows and some special objects like castle's entrance road. - TERRAIN_LAYER = 3 // roads, water flaws and cracks. Essentially everything what is a part of terrain. + OBJECT_LAYER = 0, // Common objects like mines, forest, mountains, castles and etc. They affect passability. + BACKGROUND_LAYER = 1, // Objects that still affect passability but they must be rendered as background. Such objects are lakes, bushes and etc. + SHADOW_LAYER = 2, // Shadows and some special objects like castle's entrance road. No passability changes. + TERRAIN_LAYER = 3 // Roads, water flaws and cracks. Essentially everything what is a part of terrain. No passability changes. }; struct TilesAddon From e572e867a76476e5e3710df5eae981e5a1f2997f Mon Sep 17 00:00:00 2001 From: Ihar Hubchyk Date: Sun, 25 Feb 2024 17:42:07 +0800 Subject: [PATCH 09/10] Fix copyright year --- src/fheroes2/maps/maps_tiles.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/fheroes2/maps/maps_tiles.h b/src/fheroes2/maps/maps_tiles.h index 5b9833ba69d..da1b0002bdf 100644 --- a/src/fheroes2/maps/maps_tiles.h +++ b/src/fheroes2/maps/maps_tiles.h @@ -1,6 +1,6 @@ /*************************************************************************** * fheroes2: https://github.com/ihhub/fheroes2 * - * Copyright (C) 2019 - 2023 * + * Copyright (C) 2019 - 2024 * * * * Free Heroes2 Engine: http://sourceforge.net/projects/fheroes2 * * Copyright (C) 2009 by Andrey Afletdinov * From 160d5bc4ee798b70188480ca8e2dc7a8203009ba Mon Sep 17 00:00:00 2001 From: Ihar Hubchyk Date: Sun, 25 Feb 2024 17:43:50 +0800 Subject: [PATCH 10/10] Rephrase the comment --- src/fheroes2/maps/maps_tiles.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/fheroes2/maps/maps_tiles.cpp b/src/fheroes2/maps/maps_tiles.cpp index 68f225462b8..90d96fc6b5f 100644 --- a/src/fheroes2/maps/maps_tiles.cpp +++ b/src/fheroes2/maps/maps_tiles.cpp @@ -651,7 +651,7 @@ int Maps::Tiles::getBoatDirection() const int Maps::Tiles::getOriginalPassability() const { - // Run through all objects in this tile and create the passability based on all of them. + // Run through all objects in this tile and calculate the passability based on all of them. if ( isValidReefsSprite( _mainAddon._objectIcnType, _mainAddon._imageIndex ) ) { return 0; }