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).
Details
-
Diff Detail
- Repository
- rP 0 A.D. Public Repository
- Branch
- /ps/trunk
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 1537 Build 2425: Vulcan Build Jenkins Build 2424: arc lint + arc unit
Event Timeline
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.
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.
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.
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.
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 |
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 ;)
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.
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.