Commandeering revision since it didn't close properly after commit to close it manually.

I accidentally added "Fixes #5112" which is unrelated and was closed already in changeset 24525 .
Sorry for the confusion :/

P215 kindof working now. The paint prototype need adjustment for given area and the weight value to be determined by max/min dist to border.

In D2828#121656, @smiley wrote:

I think you are focusing on the wrong detail here.

Hi @Feldfeld and thanks for your patch ;)

I'm for @Stan 's suggestion to move them to the art repo.
That way we don't distribute them with our releases but also don't destroy potential material to find out if a copyright infringement took place.
That also is in line with @elexis ' tendency AFAICT ;)

What confusion?
There are different entities that are - or at least where - fully functional. I see no reason to replace them.

While I don't agree they do not serve any purpose since one can import them into Atlas if the copyright status is unclear it's fine for me to delete them.
However, what hints are there that they may be problematic?

This is not really related to random maps and should IMO go to global scripts ;)

Commandeering this patch to close it.
Feel free to report any further issues @OptimusShepard ;)

Fixed in rP23642
Thanks for providing your your patch ;)

Good catch, @OptimusShepard ! ;)
I'll smooth the map further on larger map sizes, maybe scaling like suggested by @smiley ;)

"I mean, if you paint two times in a row certainly you shouldn't expect the state to be stored from the previous paint job?" I agree. But a constraint returns a boolean value and should return the same when checked multiple times in a row.
"this constraint achieves the purpose" What is it's purpose? For smoothly changing tree density dependent on distance to area and/or height I suggest to use a painter rather than a constraint ;)
And there are maps that have smoothly changing tree densities (without steps btw.) in the codebase already like e.g. deep_forest.js that are generated without this constraint (or a placer for it).

This constraint is not deterministic since the random number is rolled in .allows.
That means if you use this to paint trees on an area and do this over and over again you will get increasing densities though that was nowhere specified by the user.
This is not at all how a constraint should work IMO :p

I will remove this concern because it is not specific to this patch.

@"let weight = multiplier;":
Shouldn't waight start at 1 and then be reduced each iteration?

Good to go I guess, thanks for the changes! :)

I'm a bit confused about the seemingly random numbers in randomNoiseHashx and randomNoiseHashx.
(Is there a particular reason to chose them like this? Would it be useful to make them optional arguments?)

And that newline at the end of file. Otherwise it's fine ;)

After required D1630 and D1633 are added this is good to go ;)

Missing newline at the end of the file.
Otherwise I'm to tired to review this right now. Let's call it a day ;)

See IRC for improvements: http://irclogs.wildfiregames.com/2019-11/2019-11-30-QuakeNet-%230ad-dev.log starting with "12:30 < FeXoR> ...so on to D1630 I guess ^^"

Mostly good, please try to make the JSdoc phrases and comments more telling and check that last "type" check.

Could you add a screenshot as a showcase, please?
There was the rectangularSmoothToHight function that took an influence map of surrounding tiles that could also be used to create dunes. (looks like it has been removed in a cleanup frenzy though...)

I'd suggest adding a reviewer so someone gets notified of your work waiting to enter ;)

The 'raster' lib is only used by this map so far so I'd suggest to import it in the map rather than in library.js.
I didn't see this map before and so far it looks good!
Eager to test it ;)

I think it's reasonable to see this as fixed in rP22635 (as @smiley said himself in D2010) so I'm removing the audit.

This seems to introduce #5554 so I raised a concern.

I agree adding your "C++17 removing diff" would be the way to go here.
If you already created a Phabricator differential a link to that here would be nice.

Removed spaces after keys

Some more stilechanges

Removed some unneeded brackets and some stile adjustments

Removed supernumerous comata and some stile corrections

I did some performance tests.

Reference RangeOp: https://wildfiregames.com/forum/index.php?/topic/7659-rms-design/&do=findComment&comment=156822

Due to lack of progress and no reviewer agreeing with all the changes here I'll reject this revision and close it.
Parts of it can be reimplemented in split patches.
Best regards and thanks four all participant's effort!

Good point, @Angen ! :D

Winning unexpectedly by an allied wonder one didn't know existed also sounds weird to me :p

Well, that depends on the outcome of your lobby survey I guess :p

In D2044#85377, @Nescio wrote:

Increased mod support?

That can be said about any functional code. If we would add all that we basically would have to add all mods in the first place for other mods might use parts of it.

Also, currently in 0 A.D. siege engines have a pierce armour of 50 (i.e. 1-0.9^50, which is over 99%), so they're practically invulnerable already; to me that seems to be a workaround. This patch would allow making them completely invulnerable, so e.g. archers won't waste their arrows on attacking rams, but instead attack other enemy units around.

And to me it shows that our current damage system already can handle that. (The unitAI is a different story)

Well, e.g. how many wonders can be build per player? (I don't say there's a good reason to build more than one but if e.g. a map has 2 wonders for one player, what's then? That last one wouldn't be a real argument for me to object the patch - still try to think of cases that might lead to breakage ;) )

Ah, OK.
Still, both ticket's descriptions can't be true. It's either one or the other.

In D2051#85354, @Nescio wrote:

Currently the numbers of items displayed are hardcoded, both in js and xml.

Sad.

Ideally a new row of entity panels would start when the current one reaches the right end of the screen.

That would potentially fill the entire screen with icons so I object!

Unfortunately my understanding of the gui is rather limited; I have no difficulty with the xml files, but the js code is a different cup of tea.

No problem, take your time ;)

De-hardcoding displayed item numbers (e.g. 10 researches on right, 24 structures in selection panel, etc.) is outside the scope of this patch, though.

Alright. I'm just pointing out that the hardcoded number of icons means (at least) some might not be visible and how a solution could be achieved.

@"if an entity is attackable" When does this happen?

This seems to be a duplication of D1965

The fundamental system of damage in 0 A.D. has been suspect to discussions every now and then and I think it's current approach is the result of those.
So it's really something that I consider "working well".
IMO things should use it and not extend it for some edge cases by adding code that has to be maintained for little gain.

To be able to have access to more items (hero, relic and wonder icons) the way to go is IMO to make the "sidebar" scrollable if it extends the height of the available area on the screen.
That would also allow mods have any number of heroes.
Fiddling with pixels rarely really fixes space issues.

Ok, then please use this in the ticket description rather than confusing me poor soul :p

Huh? If you want some animals to be killed by one hit give them 1 health.

Thanks for the link, @Angen !
Please add the ticket and the link to the exact diff the patch you wrote build on to the description @Freagarach.

Please explain to me the concept you have in mind what should be done with this function.
I don't know this part of the code well and don't have the full source to crawl at my disposal ATM.
I see a lot of potential for this function, just by the name, to break a lot of stuff.
Let's start with:

It might be noteworthy that despite the engine differences Spring Engine supports in-game hightmap manipulation and e.g. Zero-K makes extensive use of it e.g. most projectiles deform the terrain. So looking into their code and playing a few games to see the gameplay impact might help to determine the needs and outcome.

My last proposal also doesn't work because each tile will get it's own ID and thus priority (which is neither what I wanted nor what would look decent) ;/
(At least, since the texture IDs will then be unique they would be much faster to search for ;D)

@elexis yes, just I'd use IDs. And we also need a priority "high"/"low" (I'd actually prefer paintBelow True/False, False by default to have unchanged maps).
But scrap that for now since we can use most of the code that is present and only hook things in at two points.
(Won't enable map authors to change the priorities later if placement order does not allow the desired texture order which I hoped for)

@elexis I agree with all of that but that's mainly about what's exported.
That priority array I keep talking about is just an IMO easier way to handle the shifting of e.g. one texture above all neighbor textures.
Placement methods can just receive "low" or "high" parameters (the default being high would keep existing maps as they are).

I think the usecase for basically all maps is to paint plants/grass or other textures that are "higher"/have a more uneven "surface" above flat textures like e.g. sand. (For that we don't actually need to give the map editor much control about texture priority though.)

I'm sorry for the confusion that I may have caused because this should have been a plain:
A) I agree with both of you to replace the actor/entity combination strings with objects. Just that { "texture": tForestFloor, "entity": oPalm } would do. Using simpleTerrain right away would also do in cases where there is no array.
B) In the long run (likely a later patch) the simpleTerrain and randomTerrain should be used (what was what I was talking about but is not really related to this patch, so sorry.).

Oh, and if a map doesn't want to use texture support they could be just pushed to the list I keep talking about and the resulting behavior will be as it was before this patch ;)

Removing dependencies without much duplication sounds like a good idea to me, yea.

Well, I think there are 2 types of random terrain that would be useful:

1. An array of multiple objects/associative arrays (key/value pairs) of exactly one texture and (optional) entity/actor each
2. An array of multiple textures and entities/actors to result in a random combination of those

So in the end we would have

• A "simple" terrain (one texture, no or one entity/actor)
• A random terrain (multiple textures and multiple entities/actors combined randomly)
• An array of multiple of those two (resulting in more control over terrain/texture combinations while still being random)

Thanks a lot for everyone working on this :)

I agree. My intention is just to clarify if we can find something we all agree on so we can aim for that.
And this decision has some impact on this ticket that's why I'm bringing this up.
What's you thought in this, @elexis ?

We could avoid spreading map related files over different directories by granting maps directories and put all related files there.
(This is what I would understand what @elexis called "logically grouped". Group what is only used by one map into that folder including e.g. the map-preview. There are other possible logical groupings orc..)

Dunno what the biome discussion is about, this isn't a biome, nor does it claim to be a biome, but it extends the existing biomes, which I think only these two maps currently do.

In D1660#66565, @smiley wrote:

The way I see it, this is not a biome at all.

Then don't pot it into a biome file.

This is just defining entities based on the biome.

...which is exactly what a biome does (alongside terrain textures and actors).

I do not think these should be put in to the biome files for the reason.

I can't follow that reasoning (see above). However, removing the camps and farms would be an alternative clean solution (though maybe not desired).

The next such map may not want to have Romans in the Alpine biome.

...and another map may not want to use that base texture for that specific biome. That's always a possibility. Also I'm absolutely fine with changing the entities.

So, I think that keeping this as part of the map would be best.

Could do. But then:

I find less files easier to maintain then multiple files BTW.
Also this biome is not meant to be unique to this map.

Thought I find the Obect[] syntax quite misleading for the content of the array is more prominent than the array itself this way.

Change documentation once more

(The obelisks are just meant to show the center e.g. to show where it is on fortress (the "center of mass") compared to e.g. regular polygons/circular where it's just the center of symmetry)