I like this. Cata are really strong right now and need a nerf, in my opinion. This is a good step

sure might also be enough. I basically want to hear more input on this and just picked the removal as a discussion starter.

What if it would be halfed instead of removed?

In D4960#211412, @Langbart wrote:

The order to build a field has always led them to start farming the field, and now they are looking for other construction orders that have nothing to do with fields. I don't want them to finish any construction that is in their immediate vicinity that is not a field.

See https://trac.wildfiregames.com/ticket/6748

Successful build - Chance fights ever on the side of the prudent.

Successful build - Chance fights ever on the side of the prudent.

Successful build - Chance fights ever on the side of the prudent.

Successful build - Chance fights ever on the side of the prudent.

Successful build - Chance fights ever on the side of the prudent.

Successful build - Chance fights ever on the side of the prudent.

Successful build - Chance fights ever on the side of the prudent.

1. Changed the name to Empire Maker as builder is too literal
2. Changed "armor" to "resistance" in tooltips
3. Removed "Non-elephant" as per @Langbart's suggestion

Normaly the variance is indicated with +/- instead of ±.

In D4949#211377, @vladislavbelov wrote:

In D4949#211376, @phosit wrote:

It already is accessable.

Nope, it's implementation defined. You just recieve const JS::RootedValue& librarySettings. It might be silently built or copied inside.

There is only the one implementation.

In D4949#211376, @phosit wrote:

It already is accessable.

Nope, it's implementation defined. You just recieve const JS::RootedValue& librarySettings. It might be silently built or copied inside.

In D4949#211375, @vladislavbelov wrote:

In D4949#211374, @phosit wrote:

That isn't that hard: make m_LibrarySettings public and use reporter.m_LibrarySettings.

It's not good to allow access for everyone.

It already is accessable.
If you not want to change access specifiers it will have to look like this:

auto appendLibrary = [&rq, &librariesSettings, &libraryCount](LibraryReporter& reporter)
{
	const JS::RootedValue* librarySettings{nullptr};
	reporter.Report([&](const JS::RootedValue& settings)
	{
		librarySettings = &settings;
	});
	Script::SetPropertyInt(rq, librariesSettings, libraryCount++, *librarySettings);
};

Yes this does hide data-flow but that's only inside appendLibrary.

In D4949#211374, @phosit wrote:

That isn't that hard: make m_LibrarySettings public and use reporter.m_LibrarySettings.

It's not good to allow access for everyone.

In D4949#211372, @vladislavbelov wrote:

In appendLibrary you still need to extract JS::RootedValue (JS::RootedValue isn't movable nor copyable IIRC) that means you still hides the data-flow.

That isn't that hard: make m_LibrarySettings public and use reporter.m_LibrarySettings.

(And we need runtime because some distros link with versions that are different than the runtime ones)

In D4949#211371, @phosit wrote:

Whow that changed much... Im not sure if it's for the better. I dont like the callback (it hides the data-flow)

appendLibrary(LibraryReporter{rq, "boost"}.Add("version", BOOST_VERSION));

In appendLibrary you still need to extract JS::RootedValue (JS::RootedValue isn't movable nor copyable IIRC) that means you still hides the data-flow.

Whow that changed much... Im not sure if it's for the better. I dont like the callback (it hides the data-flow)

LibraryReporter{rq, "boost"}.Add("version", BOOST_VERSION).Report(appendLibrary);
/// IMO bether would be:
appendLibrary(LibraryReporter{rq, "boost"}.Add("version", BOOST_VERSION));

In D4952#211322, @Freagarach wrote:

Sounds good, I'll try to commit today. :)

Successful build - Chance fights ever on the side of the prudent.

Successful build - Chance fights ever on the side of the prudent.

Successful build - Chance fights ever on the side of the prudent.

The string "{} ± {} ms" is used a lot it should be in a variable.

Build failure - The Moirai have given mortals hearts that can endure.

Build failure - The Moirai have given mortals hearts that can endure.

Successful build - Chance fights ever on the side of the prudent.

Successful build - Chance fights ever on the side of the prudent.

Successful build - Chance fights ever on the side of the prudent.

Build failure - The Moirai have given mortals hearts that can endure.

D4958

D4958

Change hard-coded paths.

Build failure - The Moirai have given mortals hearts that can endure.

Successful build - Chance fights ever on the side of the prudent.

Successful build - Chance fights ever on the side of the prudent.

Move players.

Fix compilation.

Sounds good, I'll try to commit today. :)

If you want to include it into the mod / appimage I don't mind (IIRC many of the maps in the mod already have that option, so ux wise it wouldn't be much of a change?). Since it's something that specifically the mp community seems to request, one could even include this in the community mod if there is one for a27. Although not sure how much sense it makes to pre-release features in that way.

cc @Itms does it count as a bugfix?

Successful build - Chance fights ever on the side of the prudent.

Successful build - Chance fights ever on the side of the prudent.

Hello, it's been a while. Noticed the issue got pushed back again. Regardless, changed the year to 2023 after @asterix 's suggestion :)

Successful build - Chance fights ever on the side of the prudent.

Successful build - Chance fights ever on the side of the prudent.

fix typo

In D4959#211302, @Stan wrote:

Do you know why they were added in the first place?

No i don't know that.

This looks alright. Do you know why they were added in the first place?

Well I suppose variants inside variants are defined in other groups :)

So are now buildings stuck with variants depending on the season? Or can there be variants inside those variants?

In D4948#211280, @andy5995 wrote:

In D4948#211278, @andy5995 wrote:

In fact I was actually thinking about integrating some of these changes into the cm2 mod (since this patch won't be included until a28). If I could get the guts of this into cm2, that may mean more people would be able to test it. What do you think?

A more efficient use of my time might be to apply this patch directly to the AppImage builds. In effect, two releases, one regular (currently an svn snapshot), and a 2nd svn snapshot with this patch applied...

Thanks for posting their thoughts. Paraphrasing myself from an IRC PM.

In D4948#211278, @andy5995 wrote:

In fact I was actually thinking about integrating some of these changes into the cm2 mod (since this patch won't be included until a28). If I could get the guts of this into cm2, that may mean more people would be able to test it. What do you think?

In fact I was actually thinking about integrating some of these changes into the cm2 mod (since this patch won't be included until a28). If I could get the guts of this into cm2, that may mean more people would be able to test it. What do you think?

@Freagarach what do you think of "Empire Builder"?

Paraphrased comments from elexis:
There are two problem with the design of this patch:

1. biome is a setting / component of random maps - this this patch intermingles the rm component with actor changes that are also present on other maps, which is not great.
2. The patch uses the filenames of biomes to decide when to switch the actors/ textures - this is very bad from a mod perspective. E.g. if you want to make your own random map that uses a custom "winter" -like biome you can't easily use this feature. You would have to copy and change all the actors as well, which is major work.

Conclusion: Better to make this a clearly separated component/ mapsetting e.g. mapActorVariant that just matches a generic string instead of a filename. e.g. winter or winterVariant. Then a modder can just use that string in the map setting to get the benefits and doesn't have to copy + change all actors.

Other thought from me:
Are there any other potential variants besides winter?

Successful build - Chance fights ever on the side of the prudent.

Successful build - Chance fights ever on the side of the prudent.

From @wowgetoffyourcellphone 's suggestion via PM, I changed it to "Empire Builder."

Successful build - Chance fights ever on the side of the prudent.

Successful build - Chance fights ever on the side of the prudent.

Sound back to cmpPlayer.

Successful build - Chance fights ever on the side of the prudent.

Successful build - Chance fights ever on the side of the prudent.

GUI fixes.

There are 30 occurences of UnitAI.StopMoving() All of those call IID_UnitMotion so nothing new here.
However It's one new call to reset running each time, probably does not matter that much although it's JS => C++.
Tracking current speed seems to prevent uneccessary updates sound like UnitMotion's responsibility though so we should not store a local value for it.