I will probably redo this in a way which couples the resource supply with the health, instead of spawning the resource on death. Otherwise you would be able to burn down a forest with D4973 and still be able to collect the wood, which makes no sense.

get it out of the queue since it's WIP

I just tested this again I I can still say that I don't see *huge* problems emerging with these values. Units tend to push each other away from resources a bit more, but the normally find a better position quickly, so no endless loops.
This could be further reduced by increasing <MinimalForce>0.3</MinimalForce> up to <MinimalForce>0.35</MinimalForce> or even higher.

In D4971#211842, @wraitii wrote:

I think you could probably bump to from 0.8 to 1.2 and it would look a little better, but this seems like a good change to me.

In D4970#211849, @wraitii wrote:

For what it's worth I don't think you can completely fix the 'problem' _and_ have smooth pathfinding. There's some level of abstraction to an RTS that makes movement somewhat unrealistic by necessity.
Just as an example, there was a thread on reddit recently about how people don't accurately count units in Age of Empires 2, because the archer units can bunch up a lot more than you'd expect. That's part of the skill of that game, and you can't really "fix" it, there's not really a problem.
Doesn't mean we can't improve on current values, but something to be aware of.

I tested the unit_pushing demo and I think it looks better. Only when building units took a bit longer to find a good position - but no endless loop as far as I could see.

In D4970#211833, @wraitii wrote:

I think you're likely to break some things here, because you're bumping everything at once, and with somewhat large changes. I would recommend just changing one thing at a time with pushing, because movement literally affects everything in the game.

In D4971#211842, @wraitii wrote:

(For what it's worth I remain convinced that we should give up on proper ship movement and just make them clip each other like Age of Empires does, or alternatively just prevent players from directly controlling ships or something.)

In D4740#211830, @Freagarach wrote:

In D4740#211826, @marder wrote:

Does this make sense gameplay wise? I occasionally do help my allies to build a forward cc for example.

Yeah, me too. So maybe the implementation should be changed, for it makes sense to have some unit that can only construct e.g. siege equipment.

In D4970#211819, @real_tabasco_sauce wrote:

Well, currently the unit pushing values allow for large armies to condense into blobs. When this happens, it's hard to tell how many units you are up against, so I think the increased pushing will be good.

point taken

Does this make sense gameplay wise? I occasionally do help my allies got build a forward cc for example.

up

In D4970#211790, @real_tabasco_sauce wrote:

@marder thanks for making this patch. I think more pushing is a welcome change, as currently armies can squish together too much. I am not sure how much pushing is needed, but I am thinking that plenty of players might call it too much pushing. Perhaps 1.25 would be better for StaticExtension, but I have little familiarity with these values and I trust your judgement.

In D4762#211768, @abian wrote:

Please don't hate me for this, but I do find the "Trained by" useful.

Could you describe a bit more how specifically it's useful?

In D4762#211734, @Stan wrote:

What if a unit is trained by a building you can't build, but you can capture?

In D4762#211731, @Stan wrote:

This probably something that one won't look at in game, but maybe people do in the structure tree?

sooo ... any more opinions on this?

update heavy warship.

half the current modifier.
Adjust tootip to be very vague

works on windows as well. no more black spots.

In D4961#211461, @chrstgtr wrote:

In D4961#211460, @marder wrote:

In D4961#211458, @real_tabasco_sauce wrote:

In my opinion, features like this should have a particular set of strengths that make them interesting, rather than ALL of the possible strengths (my thoughts on han in general tbh).

also agreement on that

Separate it out to make it a new tech? For Han or another else. I don’t care, but I generally dislike cutting things instead of balancing them

In D4961#211458, @real_tabasco_sauce wrote:

In my opinion, features like this should have a particular set of strengths that make them interesting, rather than ALL of the possible strengths (my thoughts on han in general tbh).

How about we go for <BatchTimeModifier op="mul">0.8</BatchTimeModifier> / slightly worse then half the current effect

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

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...

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

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?

update:

• betterize the iber wall check: actually pass the positions of the other player and calculate if there is enough space
• very soft deprecate the rmgen2 base placement.

In D4948#210987, @Freagarach wrote:

Some var -> let|const and let -> const are appropriate. ^^

more comments

I forgot to mention the change from sortAllPlayers() to getPlayerIDs() IDs in some places.
see D4946 for the reasoning.

the cleanup in D4670 may still be done in the future

In D4910#209709, @Stan wrote:

cc @marder,

Should I test it ?

general opinion seems to favor D4697

still think this is a fun idea, but probably better if done in a mod

need a better implementation from someone who knows more of UnitAI

meh

abandon for now

teasmPosition[i] leads to map generation failure, since you removed the for loop.
I think it could be solved by the inlines, but maybe you have a better idea?

I still very much like to include this (and last time I tried the map it looked great).

something like this should fix the wrong assignment

I generally think it is good to move the function (and rewrite it/ I like the new pattern) but it fails at some points it seems.

In D4271#204349, @Freagarach wrote:

There is something to say for storing this in the civ.json (which ought to keep only gui data of a civ) as well.

Just for my personal knowledge: Is there a correct place to store non gui civ data? Or why is civ.json only for the gui?

the last drafts in the forum look good (with columns sorted by function), but that would require much more work.

Looks good and cleaner as the version before. Only thing is that this is now civ-specific data that is not stored alongside the other civ-specific data, but rather in the naming convention.
On the other hand the civ data is already not stored in one place. Not sure if @Freagarach has a strong opinion on what the right place to store such information would be.

As everyone here seems to like this change: I will look at this again in the next time and see if it is a good idea to commit it as it is now (or if it needs more work)

In D4791#204313, @Stan wrote:

Not sure why you need to remove the variant from the actors but not the actual variant?

now that we have rP27122 (which do look really good btw!)
@wowgetoffyourcellphone should we revisit this idea but with civ specific designs?
Or do you want to commit the designs you made?

alright, build looks better now.

• reminder to myself: delete the approach variants on commit

fix the tooltips

checkrefs

In D4771#203576, @vladislavbelov wrote:

Just leave gif here:

I would say it is a precursor to that

yes and no. It does not help when you one shot a unit, but from what I've seen it looks a bit better when two larger groups of units fight against each other (as UnitAI normally also tends to target the same unit) or when you attack/ capture a building.

problem: changing the prepare time leads to bad animation glitches.
The only way to get nice looking animations (without changing the c++ part) is adding the delay to the repeat time, as there the animation is correctly adjusted to the time.
This means that the units would attack at the same time at first, but then slowly get out of clock (until they find a new target).

idea seems good, just doesn't work like this

yeah, as the description says, it's an old patch and WIP.
Just wanted to get opinions if that (trees heaving health an using the sinking animation) is something worth to pursue.

In D4771#203181, @Freagarach wrote:

A random thought: Does this mean trees lose health when hit by stray arrows?

In D4762#203174, @Freagarach wrote:

We might first want to design what we want it to look like?