remove a newline

In D5139#218678, @real_tabasco_sauce wrote:

About when are the triggers?

The water rises just after 260 seconds from "shallow" to "deep". And rises just after 1560 seconds from "deep" to "mainland only".
Both times are relative to the start of the game.

Remove the queue size limit.
Add a iteration limit.

• Update copyright year
• use const in many places

• Split and rename CMapReader::MapGeneration
• Rename GenerateMap
• Use MiB

Make the shore also independent.

Use .bind instead of arrow functions more often.
Correct a indent mistake.

We don't know what to use for macOS.

I i'm not completely mistaken "/dev/urandom" is available on macos.

So std::random_device{"/dev/urandom"} would be a better fallback then std::default_random_engine for unix like systems?

std::random_device uses "/dev/urandom" in the background. On machines where std::random_device{}.entropy() == 0 the old way of generating random numbers (querying "/dev/urandom") did likely also fail.

In D4705#218375, @vladislavbelov wrote:

Preliminary results for SVN: we have about 60% of users on macOS and 3% of users on Linux who has 0 for std::random_device{}.entropy().

I don't see how any value would be an argument against this diff.

I know that i violate this rule

• Add forgoten .bind(this)
• Inline pData

According to elexis all heights should be scaled by the map size.
For me this makes sense for hills and mountains but definitly not for waves or shores.
I'm uncertain if the sea ground height should be scaled by the map size. Conceptualy it should be treated like hills but in practice a deep see ground hurts performance and isn't noticed by players.

In D5129#218240, @real_tabasco_sauce wrote:

In D5129#218186, @phosit wrote:

1. Add a switch to Flood and make Flood better.
2. Make a new Migration, add a switch to the new Migration and then delete Flood because it's just a bad version of the new Migration.

It feels like 1. is less of a hurdle.

Well, the layouts are a bit different, mainly the tree placement and resource distribution.

"Different" isnt necesarily better or worse.

Also, I have already done the 'new migration' part : )

The bigger hurdle is decideing that we need a new map. *And* then deciding that we delete Flood.
Whereas deciding to change resource distribution of Flood is a trifle. (Adding a switch is a even smaller trifle.)

I think it might make sense to let migration stay as is written, but let the 'switch' be some probability that the water is shallow like flood (unless it could be set by the user, I'd need some help to do that).

That's exactly what I imagine: The passability should be selectable like the placements on other maps. I'd like (help) to implement the switch.

I like that flood has these kind of square hills and mountains. I am thinking about scrapping the little impassible cliffs on the current migration center island and adding in these set elevation hills.

I also hate flat mountains.

Make it a member function in CampaignMenu.

1. Add a switch to Flood and make Flood better.
2. Make a new Migration, add a switch to the new Migration and then delete Flood because it's just a bad version of the new Migration.

It feels like 1. is less of a hurdle.

The new Migration looks pretty much like Flood.
The difference is that the water is "chest deep".

RunJobs could be run in non-simulation frames. Which would make the game smoother.

I wasn't aware that you try to mimic a map from aoe2. That's an argument for this change: Player expect Migration to be similar to aoe2 and then are disapointed because the islands are that small.

Many small changes. Especially comments.

Because the bottleneck is stone/mettal instead of wood the migration will take place later.
IMO It's more interesting when the migration takes place early. Because players are be in p1 they can't build anything on the mainland and their recidence is more volatile.
Also in late game there can be markets and the player aren't forced (that much) to migrate.

In D5001#218080, @vladislavbelov wrote:

In D5001#218041, @phosit wrote:

There are many typos, you don't have to comment on thous.
I'm more interested in what you think about the last revision...

It looks ok I think. Maybe it's not ideal but it's pretty clear what's going on.

Great to see the new placement options fit in to existing map. (And it's great to see they are adopted that eagerly by map makers ;) )

There are many typos, you don't have to comment on thous.
I'm more interested in what you think about the last revision...

I didn't check if it's complete (everywhere where an error-code is printed also the string is printed)

Is this ment to be applied on top of D4634?

Remove INVALID_PROGRESS and it's uses.
I'm not sure this should be done in this diff so it's in an own revision.

Make the flags runtime values so that I was able to converge the test and non-test paths.
Also I rewrote some comments.
I'm not sure if the functions in JSI_VFS should also respect the flags... So I did the change half way ;)

• schouldn't -> shouldn't
• OPTIMAL_UTILIZATION -> MAX_QUEUE_SIZE_FOR_OPTIMAL_UTILIZATION
• Add an anonymous namespace.

On its own this is better then D4816 but it doesn't completely supersede it. D4816 also moves some GC calls to the LimitFPS.

I can't reproduce the errors on extinct volcano anymore. And it's not related.

• remove z-value from the buttons
• increas the headersize for consistency
• fix loading of savegames when the loaded mods are in a different order
• move the hidden=true attribute in the ClearSavegameButton to the xml file

As promisses enable async functions I think they are usefull without multithreading support.

When testing with my mod: when the water rizes the ships get destroied. That shouldn't be the case but it's the same behavior as before the diff.
When testing with extinct vulcano: there are a lot of errors. Again the same behavior as before the diff.

Cant you use string_VkResult from vk_enum_string_helper.h?

Fix:

• crash when starting a non saved game.
• crash when viewing the ai settings (due to reading a undeclared global variable).
• reset of the saved-ness state when changing the player assignement.

Smaller cleanups

A more minimal patch.
Less functionality (AI-player have to stay AI-player, non-AI-player have to stay non-AI-Player.)
Less code duplication
Less code changes
Less globals ;)
Less changes to the gui (I don't think moving the buttons is that useful.)

I like the your first approach. One might argue that setting the flags during runtime is not save.

A note, not a concern: Your analyzis is about the simulation (There are idle times in the server thread for example) but you also change code used in the server.

@wraitii Your error is fixed with D5060 i think.

• Rename the struct
• Remove _MapGenerator suffix
• Indent the lambda more

Should CMapReader::GeneratorState follow the rule of five?

Write a comment about the [[maybe_unused]].

We should first settle on a async design.
An event should be "awaitable". (not necesaraly but ideally C++20 co_await)

This is an improvement and the things in the test plan worked.
For the future: I think it's wrong that graphics/ knows something about the simulation. A hack would be to make the CUnit a template on a id-type.

Use optionals again. Reasoning in the previous comment.

Note that i didn't test on windows.

I changed my mind about "unswitching". Now i think it's better if the code path are unified (only one loop and switching/branching inside the loop). Bacicly this version
There where two path a visual and a non visual. With the RL::Interface I doubled the number of paths. It doesn't scale. We would have to double the number of paths everytime we introduce a new option {replay/autostart, map editor...}.
In future we should also unify Frame and NonVisualFrame.

• Rename Func to Callback
• Indent the lambdas more

Yes it's a bit less readable, but it's better then repeating while (g_Shutdown == ShutdownType::None) four times.

Unswitch the visual and the non-visual loop. This removes the need for std::optional but now there are four similar loops. I deduplicated the loop in to WhileNoShutdown.

Why didn't you commit D5102?

Change the signature of Frame to take a raw pointer.

Licence year

@vladislavbelov do you still have a concern?