Page MenuHomeWildfire Games

Make structures not block construction
ClosedPublic

Authored by temple on Apr 9 2018, 9:10 PM.

Details

Summary

An alternative plan to D1439 for overlapping wall towers preventing each other from being built is to not have structures block construction. They still block foundation so we can't place a foundation on top of another (besides walls), but now if two buildings overlap they can still be built. (This was the case in a22 too because although they had the block construction flag it was never checked.)
Suggested by elexis.

Test Plan

See that overlapping foundations can now be built.
Are there any unintended consequences?

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

temple created this revision.Apr 9 2018, 9:10 PM
Vulcan added a subscriber: Vulcan.Apr 9 2018, 10:56 PM

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

Link to build: https://jenkins.wildfiregames.com/job/differential/373/display/redirect

elexis added a subscriber: elexis.Apr 11 2018, 1:59 PM

Still seems ok on second and third sight.
Was wondering why we even have that flag if its not true in this case. But units still have it and are the primary use case now.

The schema is still correct:

Whether players should be unable to begin constructing buildings placed on top of this entity

This was the case in a22 too because although they had the block construction flag it was never checked.)

Which commit changed that?

Are there any unintended consequences?

I'm reminded of a concern on survival of the fittest for a missing condition to test for packable CCs in some mod.
IIRC it uses the Upgrade component. That uses the ObstructionsBlockingTemplateChange function from Transform.js and that tests for GetEntitiesBlockingConstruction.
So with this patch I guess it might allow unpacking if the packed entity is smaller than the unpacked one and if the packed one is touching a different obstruction.
This should be an unproblematic consequence, because the template of upgradable structures can still set the flag manually if they want to prevent ovelapping buildings.
I believe we don't have ovelapping structures currently besides walls in the public mod and we are already dealing with the pathfinding problems of that.

Suggested by elexis.

I guess that disqualifies me as a reviewer, but I would propose to commit this based on the arguments.

elexis accepted this revision.Apr 11 2018, 1:59 PM
This revision is now accepted and ready to land.Apr 11 2018, 1:59 PM

This was the case in a22 too because although they had the block construction flag it was never checked.)

Well, you must mean the D21 Foundation.js change. (The flag is checked for each entity checked, the entity itself just isn't).

In D1445#58950, @elexis wrote:

This was the case in a22 too because although they had the block construction flag it was never checked.)

Well, you must mean the D21 Foundation.js change. (The flag is checked for each entity checked, the entity itself just isn't).

Before then units were checked but static obstructions were not.

This revision was automatically updated to reflect the committed changes.

Are there any unintended consequences?

Welp, this broke rP23710

Gates were checking for entities blocking construction, which does not include neighbouring walls from RM-skirmish stuff. I changed it to checking for things that block movements, and suddenly gates from Skirmish-placeholder no longer worked. Regular gates still worked since they share an obstruction group with their neighbours.

The block-construction thing seems tricky and I can't really claim to understand it well, as I'm not very familiar with gate & wall code, so I think I'll just revert that particular part of my patch which wasn't really necessary.