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.

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

-

Event Timeline

fatherbushido created this revision.May 7 2017, 9:46 PM
fatherbushido edited the summary of this revision. (Show Details)

Fix Wall towers

mimo added a subscriber: mimo.May 8 2017, 11:09 AM

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.

Vulcan added a subscriber: Vulcan.May 8 2017, 12:12 PM

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.

Vulcan added a comment.May 8 2017, 1:08 PM

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 :/

fatherbushido updated the Trac tickets for this revision.May 8 2017, 4:04 PM
mimo added a comment.May 9 2017, 7:11 PM
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?

mimo added a comment.May 10 2017, 8:40 PM

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.

mimo added a comment.May 11 2017, 7:03 PM

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

elexis added a subscriber: elexis.May 24 2017, 7:31 PM

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

fatherbushido planned changes to this revision.May 25 2017, 9:31 AM

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.

500 -> 4000 -> 2000 -> 1200

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.

Imarok added a subscriber: Imarok.May 28 2017, 5:07 PM

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

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.

Imarok accepted this revision.May 29 2017, 8:06 PM

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
mimo added a comment.May 29 2017, 8:18 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.

mimo accepted this revision.May 29 2017, 10:21 PM

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.