Page MenuHomeWildfire Games

improve starting resource placement
Needs ReviewPublic

Authored by Baelish on Feb 22 2024, 5:41 PM.

Details

Reviewers
marder
Trac Tickets
#6605
Summary

Patch for error 6605: Failed Starting Resource Placement

Test Plan

discuss

Diff Detail

Repository
rP 0 A.D. Public Repository
Branch
/ps/trunk
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 22828
Build 56001: arc lint + arc unit

Event Timeline

Baelish created this revision.Feb 22 2024, 5:41 PM
Owners added a subscriber: Restricted Owners Package.Feb 22 2024, 5:41 PM
Baelish requested review of this revision.Feb 22 2024, 5:41 PM
sera added a subscriber: sera.Feb 22 2024, 6:37 PM

I did not remove old parts, so i took them as comments

You should remove them as they can't stay for a final commit and also so the diff actually shows your changes properly.

The summary should describe your rational for choosing this exact implementation

binaries/data/mods/public/maps/random/rmgen-common/player.js
246

English is the only permissible language for comments. Except for very rare cases comments are to be placed on the line above. The comments here are probably better left out anyway.

Baelish updated this revision to Diff 22801.Feb 22 2024, 7:15 PM
Baelish edited the test plan for this revision. (Show Details)

The basic idea for animals, berries and mines is to map the entire area surrounding the base in polar coordinates. Integer rages are taken between a minimum and a maximum radius (integers because greater precision is useless and increases the number of points to be calculated, making it slower) and an angle between 0 and 360 (in the code is in radiant) with an interval of 1 degree. In this way we will have 360 times n non-random attempts and we will be certain of finding a place in any situation. The program places the resource as soon as it finds space: for animals it starts from the smaller radius 5 (up to 12), for berries and mines from the larger one 12 (up to 7) and then scales up. This choice was made to minimize the number of attempts and reduce calculations: in the external radius it is more likely to have space, while in the internal one there is no difficulty in placing the animals, which take up less space.

Baelish updated this revision to Diff 22802.Feb 22 2024, 10:19 PM
Baelish marked an inline comment as done.

respect coding convention

sera added a comment.Feb 22 2024, 11:22 PM

Thanks for the rational, tho the idea was to update the summary at the top of this web page. The summary is what the commit message should be based on.

binaries/data/mods/public/maps/random/rmgen-common/player.js
259

there is rotateAround() for vector2d, which is cheaper than creating a new vector each time. Also the argument would be a constant, so could be calculated once only.

the incremental seems to be from some earlier iteration of the patch ...

Baelish added a comment.EditedFeb 23 2024, 1:16 AM
In D5247#223294, @sera wrote:

Thanks for the rational, tho the idea was to update the summary at the top of this web page. The summary is what the commit message should be based on.

I talked with elexis on QuakeNet for more than 2 hours. I don't know if there are logs also for private chat but anyway I will resume to you what emerged. My code in some extremely complex situation is useless, because it gives error with mines in some cases (really a few) and it's impossible to give to step angle less than 1, because it makes code very heavy.
He said that to resolve this problem "the codebase would first place all CCs of all players, then all mines of all players, then all trees of all players, then all berries of all players, then the animals. and for the animals it can be really lenient and use distance 0 or 1 to stuff", basing on some tries he did on jebel bakar map, that's one of the most problematic maps.
According to g_PlayerBaseFunctions it first places trees, then mines, then berries, then chicken. For the team on the left side, the rightmost player gets all of this stuff, then the second player gets trees, then there is no more space for mines according to the algorithm.
This map doesnt work with the resource distance = 4. Using createArea(new MapBoundsPlacer(), new TerrainPainter("red"), new NearTileClassConstraint(clBaseResource, 4)); he obtained https://i.ibb.co/LZ8qfvQ/Screenshot-from-2024-02-22-21-22-02.png
He's let the code only place one player base, deleted everything else after that (until g_Map.ExportMap): https://pastebin.com/raw/8uVNwtJS

Tomorrow I'll try a last desperate bet: 0.5 step angle and rotateAroung() to recover a bit of velocity. Now, where I live, it's too late

Feldfeld added a subscriber: Feldfeld.EditedFeb 23 2024, 6:57 AM

It is good if the base placer is more flexible.
That said, in my opinion, it should not be expected of every random map to support the most extreme settings. So if an hypothetical random map that typically has 75% its total area impassable is generated in 4v4 tiny and fails to place some features... It's fine in my opinion.
What should be done imo is that in the game settings screen, if the configuration is deemed too extreme, then a warning should appear (I'm not referring to the console but something integrated to the game settings UI) but the user should be allowed to launch the game anyway. Then if the base placer fails to place some features, it should not show an error, at best a warning (possibly more generic, appearing once, saying not everything has been placed).

edit: But yeah, if the base placer fails when it should have obviously succeeded, then it is a problem. Think I recall seeing that issue when creating my map Alpine Mountains

Stan added a reviewer: marder.Feb 23 2024, 8:36 AM
sera added a subscriber: elexis.Feb 23 2024, 11:33 AM

@Baelish , not sure I follow you. To find a solution one must exist, the code you touch doesn't guarantee such at all. What the ticket suggest is to make the radius variable to some extent to increase the probability such a solution exist in the first place. Ofc there is a tradeoff that now possible minor imbalance is introduced, like one players chickens are on average one tile further away than the other players or the like. The change from random angle to iterative angle in practice should be less impactful.

There are other possible tools like giving the map authors a tool to limit the number of players for a given map size and I'm sure someone more familiar with rmgen could surly come up with dozen more ideas than me. The point is to optimize the chance for fining a solution without compromising generating cost to much and keeping balance somewhat reasonable. After all you could always just naively restart with a different seed if you run into road block.

Looking at the ticket the author seems to believe that his suggested approach is sufficiently good enough for our need. I can only say it doesn't sound unreasonable to me. If you and @elexis can come up with something better I'm all ears.

Baelish added a comment.EditedFeb 23 2024, 11:58 AM

With elexis, we said that a good compromise between probability and resources are 15 angles of 24 degree.
I followed the ticket advice and it was really useless, beacuse it doesn't work. In addition, my first try was a random radius between max and min, according to the ticket, and when it worked, it created anyway an imbalance. There are maps, such as Pompeii, where the player 2 must have all resources very near because it spawnes on a small peninsula. So in a tiny map it's impossible to preserve the balance because the space available to each player is different (i.e. in Pompeii player 1 starts on the continental part of the map and there are not problems for him)

In D5247#223300, @sera wrote:

What the ticket suggest is to make the radius variable to some extent to increase the probability such a solution exist in the first place. Ofc there is a tradeoff that now possible minor imbalance is introduced, like one players chickens are on average one tile further away than the other players or the like.

Be careful about that. Depending on how it's done it could have an impact for competitive gameplay. Distance of berries for example is quite important as it can occasionally happen that there is a very early cav raid here and the longer it takes for women to come back to CC, the more are killed. And maybe if a resource is close enough to the CC that can remove the need to build a dropsite to collect it.
That is one reason I don't like much the approach suggested in the ticket (though it could perhaps be implemented in a way that minimizes the impact on balance if conditions are good to start with e.g. enough space). This is why imo we don't necessarily need to support placements where there is barely space around CC and it comes back to the suggestion I made earlier.

sera added a comment.Feb 23 2024, 7:15 PM

Depending on how it's done it could have an impact for competitive gameplay.

Which also might be a good thing. You will have to scout before you know the value of a cav rush or take your chances then. Excessive balancing can lead to a boring game. Just imagine all civs being the same except for names and textures. If berries a tile or two further away can decide a game I'd say it's not the map script that needs fixing ;)

Anyway I'm only in this discussion for code review and have no relevant opinion on starting base layout, so I wont root for or against it.

In D5247#223305, @sera wrote:

Anyway I'm only in this discussion for code review and have no relevant opinion on starting base layout, so I wont root for or against it.

Yeah I imagined. Even though I quoted you I intended my argument to be directed to everyone. I made my opinion since reading the ticket already.

In D5247#223305, @sera wrote:

Which also might be a good thing. You will have to scout before you know the value of a cav rush or take your chances then. Excessive balancing can lead to a boring game. Just imagine all civs being the same except for names and textures. If berries a tile or two further away can decide a game I'd say it's not the map script that needs fixing ;)

Now while I know you have no strong opinion on that matter and made that argument for the sake of it... I have to give it a proper answer now.

Excessive balancing can lead to a boring game. Just imagine all civs being the same except for names and textures.

I disagree that the analogy is applicable in this situation. For me, all civs being the same except for names and textures is similar to the "symmetrical maps" which have been suggested to me here https://code.wildfiregames.com/D4232#201256 and I argued that would be boring. I believe it is possible to have diverse maps while not sacrificing balance on something as basic as the starting base layout.

Which also might be a good thing. You will have to scout before you know the value of a cav rush or take your chances then.

For me, the game would be the most interesting if scouting was worthwhile for searching the opponent's strategy and the precise location of his resources (random but balanced). If it's searching for imbalances like starting berries being far enough for that very early cav rush to be viable, effectively making this into a game where a player has one more strategical option than the other, then imo it's not a good thing. I feel like I already argued somewhere about that "scouting for strategy" topic but I can't find it. And as you said there's the "taking the gamble" aspect. If the balance is bad to the point where taking the gamble can bring you close to a 50% chance a victory then a lower rated player can abuse it when he plays against a stronger player for example.

If berries a tile or two further away can decide a game I'd say it's not the map script that needs fixing ;)

Well berries can already be spawned in various group shapes. If even with that they fail to be generated I fear that it could take more than that to fix the issue.
And anyway, to balance a RTS for competitive gameplay some things are more sensible than you'd think. In the thread I linked I mentioned AoE2 and I think now it remains a good comparison. Even in their "MegaRandom" map they have balanced resources. And I believe right now, for their competitive maps, they do in fact balance down to the exact tile distance with respect to the CC/TC. I'm maybe going a little off-topic here, but as far as map design goes in my Mainland Balanced map I strive for random but balanced resource amount (so you don't know how much you will have before starting the game, but you know you will have the same amount as your opponent, something that AoE2 doesn't do). I let some variability w.r.t. distance to CC for resources other than the starting base, something that AoE2 doesn't do. I even allow different hunt/berry ratio distribution for players in the same game (doing that in AoE2 would automatically put your map in the trash bin as far as competitiveness is concerned). Just to show that I do keep diversity very much in mind and try to avoid "excessive balancing"

Also, I don't consider the current starting base layout to be set in stone. It's true that it looks kinda ugly and unnatural as it is. I already had ideas of turning some random maps into the Delenda Est style where every starting resource is further away from the CC, which would remove most of the current starting base features. But this restricts even more the extreme map settings, and for implementing something like this, considering starting resources would need to be balanced while integrating well with the rest of the map, the current rather simple starting base placer would not be enough.
At least the current starting base has the advantage of giving some form of starting balance even ignoring the rest of the map.

Be careful about that. Depending on how it's done it could have an impact for competitive gameplay. Distance of berries for example is quite important as it can occasionally happen that there is a very early cav raid here and the longer it takes for women to come back to CC, the more are killed. And maybe if a resource is close enough to the CC that can remove the need to build a dropsite to collect it.

I understand what you mean, but the code starts on the same radius for each player and it stop iteration if there is enough space. You can see the difference only if some players have got big disadvantages with their space, so it can be a rebalancing to put resources nearer. In addition, I want to underline that code searches all around the base, so if a player has got all the external radius full, is strongly disadvantaged.

Feldfeld added a comment.EditedFeb 24 2024, 10:58 PM

Be careful about that. Depending on how it's done it could have an impact for competitive gameplay. Distance of berries for example is quite important as it can occasionally happen that there is a very early cav raid here and the longer it takes for women to come back to CC, the more are killed. And maybe if a resource is close enough to the CC that can remove the need to build a dropsite to collect it.

I understand what you mean, but the code starts on the same radius for each player and it stop iteration if there is enough space. You can see the difference only if some players have got big disadvantages with their space, so it can be a rebalancing to put resources nearer. In addition, I want to underline that code searches all around the base, so if a player has got all the external radius full, is strongly disadvantaged.

Yes, if it only has an effect when there is a clear lack of space then I don't have complaints as it shouldn't change competitive gameplay (if the starting radius is the same as previously). That would make the patch good as a standalone though imo it would still not close the ticket for the reason I mentioned earlier (not every map should necessarily support the most extreme settings).

Yes, if it only has an effect when there is a clear lack of space then I don't have complaints as it shouldn't change competitive gameplay (if the starting radius is the same as previously). That would make the patch good as a standalone though

I agree with you here

imo it would still not close the ticket for the reason I mentioned earlier (not every map should necessarily support the most extreme settings).

and here, and I want to add that elexis tested some maps with this extreme settings and it would not work anyway. so in my modest opinion, the best choice is: use my method with less extreme settings and for those maps, that don't work, use a UI banner, as you said previously.

sera added a comment.Feb 25 2024, 1:13 PM

Ok, now that we have established that for sufficiently small deviation in radius this approach is acceptable let's continue getting the patch into shape.

@Baelish, you state it doesn't work, guess you don't mean the currently published version as that one is obviously bugged but that this approach doesn't solve all cases, which is to be expected I'd say. Would sorting the "distance passes" with r the current radius like r, r+1, r-1, r+2 ... improve the result?

With elexis, we said that a good compromise between probability and resources are 15 angles of 24 degree.

This has some tendency to clump compared to the original approach, guess using and angle of something like 6*PI/17 instead would be a further improvement.

marder retitled this revision from Error 6605 to improve starting resource placement.Feb 25 2024, 5:08 PM
marder edited the test plan for this revision. (Show Details)
marder updated the Trac tickets for this revision.