Page MenuHomeWildfire Games

Let wall being captured by territory but not by capture attack.
ClosedPublic

Authored by fatherbushido on May 7 2017, 9:46 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Oct 10, 1:16 PM
Unknown Object (File)
Thu, Oct 10, 1:16 PM
Unknown Object (File)
Thu, Oct 10, 1:16 PM
Unknown Object (File)
Thu, Oct 10, 1:16 PM
Unknown Object (File)
Thu, Oct 10, 1:16 PM
Unknown Object (File)
Thu, Oct 10, 1:16 PM
Unknown Object (File)
Thu, Oct 10, 1:15 PM
Unknown Object (File)
Thu, Oct 10, 1:15 PM
Tokens
"Like" token, awarded by elexis."Love" token, awarded by Imarok.

Details

Summary

Let the Capture component for walls (it should perhaps be tweak).
Adding the walls as restricted class for the capture attack (it was bugged before #4220).

Test Plan

-

Diff Detail

Repository
rP 0 A.D. Public Repository
Branch
/ps/trunk
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 1635
Build 2579: Vulcan BuildJenkins
Build 2578: arc lint + arc unit

Event Timeline

fatherbushido edited the summary of this revision. (Show Details)

Fix Wall towers

Nice approach, but i'm a bit worried by the current state of the AI, as i'm not sure RestrictedClasses are used everywhere they should be. Did you check with AIs and walls that AIs do not try to capture walls. (maybe making a new simple ticket for checking that canAttack is tested everywhere the AI wants to attack/capture would help).
Then, as you said, some tweaks are needed: for example, i would have territory decayRate <= garrisonRegenRate so that walls do not decay when garrisoned.
In addition, fields already are not capturable and decay to territory, but with a different approach: they have no Capturable component and decay immediately to the territory. It may be nice to unify both approaches, possibly with this new one as it allows a progressive decay.

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/1068/ for more details.

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/1069/ for more details.

In D450#18421, @mimo wrote:

Nice approach, but i'm a bit worried by the current state of the AI, as i'm not sure RestrictedClasses are used everywhere they should be. Did you check with AIs and walls that AIs do not try to capture walls. (maybe making a new simple ticket for checking that canAttack is tested everywhere the AI wants to attack/capture would help).
Then, as you said, some tweaks are needed: for example, i would have territory decayRate <= garrisonRegenRate so that walls do not decay when garrisoned.
In addition, fields already are not capturable and decay to territory, but with a different approach: they have no Capturable component and decay immediately to the territory. It may be nice to unify both approaches, possibly with this new one as it allows a progressive decay.

I was secretly hoping that you will look at that ;-)
Indeed I didn't look at the AI :/

In D450#18421, @mimo wrote:

Nice approach, but i'm a bit worried by the current state of the AI, as i'm not sure RestrictedClasses are used everywhere they should be. Did you check with AIs and walls that AIs do not try to capture walls. (maybe making a new simple ticket for checking that canAttack is tested everywhere the AI wants to attack/capture would help).
Then, as you said, some tweaks are needed: for example, i would have territory decayRate <= garrisonRegenRate so that walls do not decay when garrisoned.
In addition, fields already are not capturable and decay to territory, but with a different approach: they have no Capturable component and decay immediately to the territory. It may be nice to unify both approaches, possibly with this new one as it allows a progressive decay.

I was secretly hoping that you will look at that ;-)
Indeed I didn't look at the AI :/

I will have a quick look at the AI code in the next days, checking if i see something obviously missing, but won't have time to test the Ai's behaviour with this patch :-(
That said, if somebody notices problems and provides a replay file to reproduce them, i could look at it.

I will have a quick look at the AI code in the next days, checking if i see something obviously missing, but won't have time to test the Ai's behaviour with this patch :-(
That said, if somebody notices problems and provides a replay file to reproduce them, i could look at it.

Thanks, so if there is no major issue with AI, you think that it's worth to dig that approach and to do the same for fields?

D473 implements changes needed by the AI for this patch, although it may not be sufficient. This has still to be tested.

Thanks, so if there is no major issue with AI, you think that it's worth to dig that approach and to do the same for fields?

I think it would be better if both use the same mechanism (concerning fields, having a decay which takes some time may be better that the instantaneous current one). The approach in this patch seems appealing, although i've not tried it in game: maybe the gui should be improved as having a capture bar while the structure is not capturable can be misleading for beginners.

Tweak a bit number as a draft. Attempt to do the same thing for fields.

As you expected there is a gui issue for field. We get the 3 bars (hp, res, cp) when selecting on the game screen but res and cp bars overlap in the entity panel tooltip.
I wonder if it's the way to go.

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/1136/ for more details.

As you expected there is a gui issue for field. We get the 3 bars (hp, res, cp) when selecting on the game screen but res and cp bars overlap in the entity panel tooltip.
I wonder if it's the way to go.

yes, the gui seems to be the limiting factor, so forget my proposition to also do it for fields :) not worth the trouble

Might be a bit weird to have things convert from one player to another without being able to influence it with units. But why not.

Walls should decay very very slowly. With this patch they're gone in 100 seconds as they only have 500CP. I'd change to maybe 4000CP. A fortress has 5000CP. These should be the last remains of a civilization.

What about siege walls? They are meant to be built in enemy territory. Now the player will have to pay with population cost to build these siege walls as they were used historically and garrison a unit everywhere. Wall pieces that weren't garrisoned convert to the enemy, then the walls / infantry on the walls attack that converted wall piece. Guess that is acceptable if it's sufficiently slow.

While we're at a patch changing how fields are capturable by territory, is it possible to not let fields decay by territory of allies? Recently an ally besides me reached age 3, instacaptured my fields and deleted them. Other buildings don't decay either (except building 1 tower or fort near an ally CC, then that CC slowly becomes gaia for any reason).

Tested with D516, works nicely.

binaries/data/mods/public/simulation/templates/template_unit_hero.xml
13

If I'm not mistaken, siege walls inherit StoneWall, but nice to make it explicit

500 -> 4000 -> 2000 (games are not so long)

Executing section Default...
Executing section Source...
Executing section JS...
Executing section XML GUI...
Executing section Python...
Executing section Perl...

http://jw:8080/job/phabricator_lint/70/ for more details.

Executing section Default...
Executing section Source...
Executing section JS...
Executing section XML GUI...
Executing section Python...
Executing section Perl...

http://jw:8080/job/phabricator_lint/71/ for more details.

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...

http://jw:8080/job/phabricator/1394/ for more details.

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...

http://jw:8080/job/phabricator/1395/ for more details.

elexis and I concluded 1200 will be better ;)

In D450#22283, @elexis wrote:

While we're at a patch changing how fields are capturable by territory, is it possible to not let fields decay by territory of allies?

Isn't that already the case with this patch?

Executing section Default...
Executing section Source...
Executing section JS...
Executing section XML GUI...
Executing section Python...
Executing section Perl...

http://jw:8080/job/phabricator_lint/73/ for more details.

While we're at a patch changing how fields are capturable by territory, is it possible to not let fields decay by territory of allies?

Isn't that already the case with this patch?

It should be fine now

screenshot0555.png (1×1 px, 4 MB)

binaries/data/mods/public/simulation/templates/template_unit_hero.xml
13

Yes I wondered if we should not remove that Classes from the siege wall template but it would perhaps lead to issue with the AI

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...

http://jw:8080/job/phabricator/1397/ for more details.

Assuming the AI was fixed in D473, this patch is good.

This revision is now accepted and ready to land.May 29 2017, 8:06 PM
In D450#23639, @Imarok wrote:

Assuming the AI was fixed in D473, this patch is good.

As i said in D473, in that patch, i've only modified what looked necessary to me through inspection of the code, but i may have missed some things and i think we still need someone to test the patch in game: i.e. doing a few games with the patch and checking that the AI won't try to capture fields or walls.

I test an ai ffa (about 45 min) on the fortress maps (the one with walls) and didn't notice anything wrong.
Was perhaps not a relevant test.

Thanks for the tests. I just wanted to be sure that some games with the patch have been run (i had no time to do them myself). I guess it will be more tested once commited, during the FF period.

In D450#23667, @mimo wrote:

Thanks for the tests. I just wanted to be sure that some games with the patch have been run (i had no time to do them myself). I guess it will be more tested once commited, during the FF period.

Thanks for the warnings!

This revision was automatically updated to reflect the committed changes.