Random terrain generation #8600

idshibanov · 2024-05-03T03:40:18Z

Debug feature only. Basic editor usage with F5/F6 keybinds to assist with mapmaking. More to come soon™.

github-actions

⚠️ Clang-Tidy found issue(s) with the introduced code (1/2)

src/fheroes2/maps/map_generator.cpp

github-actions

⚠️ Clang-Tidy found issue(s) with the introduced code (2/2)

src/fheroes2/maps/map_generator.h

src/fheroes2/maps/map_object_info.cpp

src/fheroes2/editor/editor_interface.cpp

github-actions

⚠️ Clang-Tidy found issue(s) with the introduced code (1/1)

src/fheroes2/maps/map_generator.cpp

github-actions

⚠️ Clang-Tidy found issue(s) with the introduced code (1/1)

src/fheroes2/maps/map_generator.cpp

src/fheroes2/maps/map_generator.h

…andom-map-generator # Conflicts: # src/fheroes2/editor/editor_interface.cpp

github-actions

⚠️ Clang-Tidy found issue(s) with the introduced code (1/1)

github-actions · 2024-05-21T03:22:53Z

src/fheroes2/maps/map_generator.cpp

+        NodeCache( int32_t width, int32_t height )
+            : mapSize( width )
+            , outOfBounds( -1 )
+            , data( width * height )


⚠️ bugprone-implicit-widening-of-multiplication-result ⚠️
performing an implicit widening conversion to type std::vector::size_type (aka unsigned long) of a multiplication performed in type int

github-actions · 2024-05-21T03:22:53Z

src/fheroes2/maps/map_generator.cpp

+    {
+        const fheroes2::Point tilePos = tile.GetCenter();
+        const auto & objectInfo = Maps::getObjectInfo( groupType, type );
+        if ( canFitObject( data, objectInfo, tilePos, true ) && putObjectOnMap( mapFormat, tile, groupType, static_cast<uint32_t>( type ) ) ) {


⚠️ bugprone-narrowing-conversions ⚠️
narrowing conversion from uint32_t (aka unsigned int) to signed type int32_t (aka int) is implementation-defined

github-actions · 2024-05-21T03:22:53Z

src/fheroes2/maps/map_generator.cpp

+        // Step 4. We're ready to save the result; reset the current world first
+        world.generateForEditor( width );
+        mapFormat.tiles.clear();
+        mapFormat.tiles.resize( width * height );


⚠️ bugprone-implicit-widening-of-multiplication-result ⚠️
performing an implicit widening conversion to type std::vector::size_type (aka unsigned long) of a multiplication performed in type int

ihhub

Hi @idshibanov , I left multiple comments and suggestions here. Could you please take a look at them once you have time?

fheroes2-vs2019.vcxproj

src/fheroes2/maps/map_generator.h

src/fheroes2/maps/map_generator.cpp

ihhub · 2024-05-25T02:06:05Z

src/fheroes2/maps/map_generator.cpp

+    const std::vector<int> playerStartingTerrain = { Ground::GRASS, Ground::DIRT, Ground::SNOW, Ground::LAVA, Ground::WASTELAND };
+    const std::vector<int> neutralTerrain = { Ground::GRASS, Ground::DIRT, Ground::SNOW, Ground::LAVA, Ground::WASTELAND, Ground::BEACH, Ground::SWAMP, Ground::DESERT };
+
+    std::vector<fheroes2::Point> directionOffsets = { { 0, -1 }, { 1, 0 }, { 0, 1 }, { -1, 0 }, { -1, -1 }, { 1, -1 }, { 1, 1 }, { -1, 1 } };


This can be std::array<> as well. Plus it can be const.

For some reason array doesn't play nicely with Point class allocation. I get no suitable constructor exists to convert from "int" to "fheroes2::PointBase2D<int32_t>"

ihhub · 2024-05-25T02:28:56Z

src/fheroes2/maps/map_generator.cpp

+        }
+    }
+
+    bool putObjectOnMap( Map_Format::MapFormat & mapFormat, Tiles & tile, const ObjectGroup groupType, const int32_t objectIndex )


It is the same as EditorInterface::_setObjectOnTile() method. We should reuse the existing code.

Similar situation as in previous comment. For example this call doesn't recalculate passabilities after every call.

ihhub · 2024-05-25T02:31:04Z

src/fheroes2/maps/map_generator.cpp

+        return false;
+    }
+
+    bool placeCastle( Map_Format::MapFormat & mapFormat, NodeCache & data, Region & region, int targetX, int targetY )


placeCastle and placeMine have the same code as editor_interface.cpp file. In the future we might even change the code there. We need to avoid using code duplication.

We can move existing functions from editor_interface.cpp file to a new file. We can call it as editor_map_helper.h|cpp and use the functions here and in editor_interface.cpp file

I agree that this is the worst case of duplication, but can't think of a clean way to extract it. I'd have to pass 2-3 function callbacks into it for object placement check, action object check an additional marking call for the generator. Editor version also have to handle error messages for the user feedback but generator just tries another time.

ihhub · 2024-05-25T02:34:51Z

src/fheroes2/maps/map_generator.cpp

+    {
+        const auto & node = Rand::Get( region._nodes );
+        Maps::Tiles & mineTile = world.GetTiles( node.index );
+        const int32_t mineType = fheroes2::getMineObjectInfoId( resource, mineTile.GetGround() );


We should use ObjectInfo from mapFormat which already contains mineType. See EditorPanel::getMineObjectProperties() for this matter.

I don't see how getMineObjectProperties would help here. I'm going in opposite direction - not having ObjectInfo I need to find out which index contains what I need. ObjectInfo groups have arbitrary order and no enums to support it, so I had to make this function.

ihhub · 2024-05-25T02:37:10Z

src/fheroes2/editor/editor_interface.h

@@ -153,6 +153,11 @@ namespace Interface
        int32_t _selectedTile{ -1 };
        int32_t _tileUnderCursor{ -1 };

+#if defined( WITH_DEBUG )
+        uint32_t _playerCount = 2;


Should we store Configuration object instead of these 2 members?

ihhub · 2024-05-25T02:38:12Z

src/fheroes2/editor/editor_interface.cpp

+                    if ( Dialog::SelectCount( "Pick player count", 2, 6, newCount ) ) {
+                        _playerCount = newCount;
+                    }
+                    newCount = _regionSizeLimit;


I am not sure that region size is an understandable entity for map makers. Can we come up with something more user friendly metric? :)

I'm fine with renaming it but can't think of more suitable name.

Should we attach the number of regions to the number of players minus the size of water on map? I assume we don't generate water area for now which is easier. In this case we won't need this parameter.

ihhub

Hi @idshibanov , I left few more comments here. Also, some comments are still valid form the previous review. Could you please take a look at them?

ihhub · 2024-05-25T07:09:29Z

VisualStudio/fheroes2/sources.props

@@ -200,6 +200,7 @@
    <ClCompile Include="src\fheroes2\maps\ground.cpp" />
    <ClCompile Include="src\fheroes2\maps\map_format_helper.cpp" />
    <ClCompile Include="src\fheroes2\maps\map_format_info.cpp" />
+    <ClCompile Include="src\fheroes2\maps\map_generator.cpp" />


I think we should rename these files into random_map_generator.h|cpp to highlight randomness. Or at least map_random_generator.h|cpp if we keep the format.

ihhub · 2024-05-25T07:10:08Z

src/fheroes2/editor/editor_interface.cpp

+                    rmgConfig.playerCount = _playerCount;
+                    rmgConfig.regionSizeLimit = _regionSizeLimit;
+
+                    if ( Maps::Generator::generateWorld( _mapFormat, rmgConfig ) ) {


Essentially, we are generating map so would generateMap be a better name?

ihhub · 2024-05-25T09:49:18Z

src/fheroes2/editor/editor_interface.cpp

+                    if ( Dialog::SelectCount( "Pick player count", 2, 6, newCount ) ) {
+                        _playerCount = newCount;
+                    }
+                    newCount = _regionSizeLimit;


Should we attach the number of regions to the number of players minus the size of water on map? I assume we don't generate water area for now which is easier. In this case we won't need this parameter.

ihhub · 2024-05-25T09:50:15Z

src/fheroes2/gui/ui_map_object.h

@@ -35,4 +35,5 @@ namespace fheroes2
    Sprite generateTownObjectImage( const int townType, const int color, const int groundId );

    int32_t getTownBasementId( const int groundType );
+    int32_t getMineObjectInfoId( const int resource, const int groundType );


Please add an empty line to differentiate 2 functions.

ihhub · 2024-05-25T09:51:00Z

src/fheroes2/maps/map_generator.h

+ *   59 Temple Place - Suite 330, Boston, MA  02111-1307, USA.             *
+ ***************************************************************************/
+#pragma once
+#include <cstdint>


Please add an empty line to separate file header inclusion list.

ihhub · 2024-05-25T09:51:45Z

src/fheroes2/maps/map_generator.cpp

+namespace
+{
+    // ObjectInfo ObjctGroup based indicies do not match old objects
+    const int neutralCOLOR = 6;


COLOR part should be Color :)

idshibanov added 12 commits May 1, 2024 15:00

Rough code for terrain generation

d8fd07a

Generate circular map design

0ba9299

Add a dialog to control map settings

168f01a

Test castle placement

2b26e6b

Mine random placement

7359ad2

Move map generation code into its own namespace

3260648

Clean up shared code

46a8a9b

Detach map region structs from generator

992c645

Define debug-only hotkeys

e766d26

Fix map border rendering

74b0a0f

Region color placement and general improvements

7185ecc

Improve player starting terrain selection

82c8a1b

github-actions bot assigned idshibanov May 3, 2024

github-actions bot reviewed May 3, 2024

View reviewed changes

Code style fixes

e021218

idshibanov added improvement New feature, request or improvement editor Map editor related stuff labels May 3, 2024

github-actions bot reviewed May 3, 2024

View reviewed changes

src/fheroes2/maps/map_generator.cpp Outdated Show resolved Hide resolved

src/fheroes2/maps/map_generator.cpp Outdated Show resolved Hide resolved

github-actions bot reviewed May 3, 2024

View reviewed changes

src/fheroes2/maps/map_generator.cpp Outdated Show resolved Hide resolved

src/fheroes2/maps/map_generator.cpp Outdated Show resolved Hide resolved

src/fheroes2/maps/map_generator.cpp Outdated Show resolved Hide resolved

src/fheroes2/maps/map_generator.h Outdated Show resolved Hide resolved

idshibanov force-pushed the random-map-generator branch from 79e57c3 to cac83d6 Compare May 3, 2024 04:16

IWYU

14e8178

idshibanov force-pushed the random-map-generator branch from cac83d6 to 14e8178 Compare May 3, 2024 04:17

Fix release build

9a74040

ihhub added this to the 1.2.0 milestone May 3, 2024

idshibanov added 4 commits May 19, 2024 00:27

Merge branch 'master' of https://github.com/ihhub/fheroes2.git into r…

11bd876

…andom-map-generator # Conflicts: # src/fheroes2/editor/editor_interface.cpp

Add a node cache struct

9df56bf

Refactor region expansion

3c56fdc

Fix object placement logic

a7d0fc0

idshibanov force-pushed the random-map-generator branch from aa973d6 to a7d0fc0 Compare May 21, 2024 03:22

github-actions bot reviewed May 21, 2024

View reviewed changes

Headers

7a8fa85

idshibanov requested a review from ihhub May 24, 2024 23:37

ihhub requested changes May 25, 2024

View reviewed changes

Basic feedback

e443648

ihhub requested changes May 26, 2024

View reviewed changes

ihhub marked this pull request as draft July 7, 2024 06:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Random terrain generation #8600

Random terrain generation #8600

idshibanov commented May 3, 2024 •

edited

Loading

github-actions bot left a comment

github-actions bot left a comment

github-actions bot left a comment

github-actions bot left a comment

github-actions bot left a comment

github-actions bot May 21, 2024

github-actions bot May 21, 2024

github-actions bot May 21, 2024

ihhub left a comment

ihhub May 25, 2024

idshibanov May 25, 2024

ihhub May 25, 2024

idshibanov May 25, 2024

ihhub May 25, 2024

idshibanov May 25, 2024

ihhub May 25, 2024

idshibanov May 25, 2024

ihhub May 25, 2024

ihhub May 25, 2024

idshibanov May 25, 2024

ihhub May 25, 2024

ihhub left a comment

ihhub May 25, 2024

ihhub May 25, 2024

ihhub May 25, 2024

ihhub May 25, 2024

ihhub May 25, 2024

ihhub May 25, 2024

Random terrain generation #8600

Are you sure you want to change the base?

Random terrain generation #8600

Conversation

idshibanov commented May 3, 2024 • edited Loading

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot May 21, 2024

Choose a reason for hiding this comment

github-actions bot May 21, 2024

Choose a reason for hiding this comment

github-actions bot May 21, 2024

Choose a reason for hiding this comment

ihhub left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ihhub left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

idshibanov commented May 3, 2024 •

edited

Loading