Page MenuHomeWildfire Games

Remove "domestic" attribute for wolves on polar sea
Needs ReviewPublic

Authored by Dunedan on May 30 2020, 6:29 PM.
This revision needs review, but there are no reviewers specified.

Details

Reviewers
None
Summary

Wolves on polar sea don't use the usual "arctic wolf" template, but a separate template which changes two properties: It makes the wolves violent, but also marks them as "domestic", which results in units not being able to fight them with ranged attacks (buildings however do fight them with ranged attacks). That's odd as it's a different behavior how units approach these wolves, compared to wolves on all other maps.

This change removes the "domestic" attribute from the special wolves type for polar sea, to make the interaction with those wolves more natural. As a result of these changes this map might have gotten a bit easier, however I did some testing and it wasn't that much noticeable.

Test Plan

Check that wolves on polar sea can be attacked by ranged units now and the balancing of the wolves in this map still feels right.

Event Timeline

Dunedan created this revision.May 30 2020, 6:29 PM

Adding @FeXoR for RMGEN and review and @Nescio for template

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

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/2267/display/redirect

elexis added a subscriber: elexis.May 30 2020, 8:29 PM

There are two questions which were not asked in the summary so far which also are the same question - why are they domestic units if that is odd? If there is something that is odd then the revision history can give you insight on why a commit added the change. So the second question which is the same as the first question is what the revision history and comments posted on that commit said.

The answer is the first revision of the map changed UnitAI to make animals controlable, then that was complained at with a reference to a some unitAI designplan which envisioned a custom AnimalAI and a notion that being able to task wild animals is just wrong - unless those are domestic units. So then the UnitAI change was reverted and to keep the wolves controlable (see use by the map TriggerScript), they were made Domestic units which also introduced the OP 60(+) slaughter attack which made them very easy to kill.
Then the same(?) "make animals controlable" feature was wished for on the forums and introduced as a separate patch elsewhere again, I think it's on Freagarachs authored patch list on Phabricator, I don't know if it's committed.
Last time I discusseded it was on 2017-03-30-QuakeNet-#0ad-dev.log and 2017-04-07-QuakeNet-#0ad-dev.log after that I resigned on this subject.
The references to commits and review comments can be found in the revision history on Phabricator of the trigger-script and mapfile (If more information is wanted on the topic, one should also be able to locate wowgetoffyourcellphones forum posts).

Sorry, but I don't get your point. Clearly non-domestic animals are as controllable right now as are domestic animals. So what's the problem with this patch from your point of view?

What @elexis presumably means is that the Domestic class is there for a reason and has more than one function in game (see UnitAI). The current situation might be ugly, but your proposed change is probably not an improvement.

I meant what I said - when someone uploads a patch removing something odd, then one should attempt to find the reason why it's odd. Often there is a reason for odd things being done and removing the odd thing may remove the oddness but also remove something intentional that the remover might not have intended to remove.
From the summary I can't distinguish whether you were aware that the wolf receives move-orders from the trigger script, which was the reason UnitAI was changed originally and then later changed to Domestic unit.

I never said or implied that I have problem with the patch, I mentioned that the summary and the test plan lack an important bit of information.
A summary for a patch that removes an oddity should mention why the oddity was introduced, why it can be removed now without removing the features (or justifying the removal of the feature), and the test plan should mention how it can be confirmed that everything works as intended.

I haven't followed the state of the game in a while. You mentioned that non-domestic units are clearly controllable now. I've tested r23702 and they are not in that revision. Also the mentioned patch to make non-domestic animals controlable is not committed (which also doesn't mean that I have a problem with the patch, since anyone can also argue the map concept is too odd to be implemented).

The comments without parentheses will be understood as helpful by most readers.

I meant it should happen this way:

  • Player plays the map, notices wolves are unexpectedly instakilled with slaughter attack
  • Player toggles developer mode, looks at the map code and sees the wolf is Domestic
  • Developer looks up revision history and finds, i.e. uses svn log or https://code.wildfiregames.com/source/0ad/history/ps/trunk/binaries/data/mods/public/maps/random/polar_sea_triggers.js and finds the unfortunate UnitAI code comment at D156#6746, the unfortunate Domestic class workaround in rP19323 triggering the issue.
  • Developer performs phabricator search for existing proposed patches on the topic, uses the keyword "domestic wolf" and finds D1960 as the second match of open revision proposals and sets it as a dependency or tries to get that one committed by scouting and talking to potential reviewers.
  • (Optional step: Developer notices noone reviews a patch, dedicates everything to getting patches committed; finally performs the reviews and QA noone else does, ultimately becomes a staff member with these inherent differences leading to disputes, ragequits, bans and powerplay while leaving people are being superseded by unsuspecting newcomers so that the cycle completes when another player notices that the wolf is getting killed with slaughter attack.)
  • ((Optinal step: But the new developer is more wise and learns about these mistakes from previous generations before repeating them, gets some wolf patch committed without getting banned and making people quit, fixes review queue, release-, gamedesign-, team, funding and PR process while making everyone else happy who has mood as the sole currency and they all lived happily even after.))

(The comments in parentheses will be coined as unconstructive by the shadow government and thus become legal banreason when in fact the comments are constructive because they allow people to navigate around mantraps in the future. Yes, I rewrote and re-read every sentence 5 times and am aware that this will be referred to as another banreason and I rather point out problems than counterproductively portraying a false image and playing happy. Since it's not obvious to people that playing happy is making things worse: If one pretends that there aren't any problems then the problems will silently grow worse which in turn increases the fall height. And none of this offtopic because it couldn't be any more on topic with the domestic wolf code. The second reason why the comments in parentheses actually are constructive is because they allow the wise developer to create a possibly preferable Animal AI. Unlikely to be me though. I suppose the third way why this is not an unconstructive comment is it providing me an anticipatory feedback channel for the purpose of rectification of discussions about me Im excluded to.)

So if I was to recommend the author or reviewer something, it would be to take the effort to comprehend the AnimalAI plans referred to and see if they meet the whatever defined software requirements and practical constraints, coming to the educated conclusion that this, or the original UnitAI code, or D1960 and to be combined with the patch or similar are correct (or convincing sufficient people to become policy to ignore existing software design plans) and find a staff member to commit.
(And if that is laid out as making contributors leave by putting up too high expectations then change the objectives of the organization to be an easy-contribution process, not a triple-A rated game dev process and post in these review posts relating to the domestic wolf etc.)
(Catch 22 is that it can make people leave by discovering the differences between easy contribution and triple A rated game in which case I can just as well stop posting which is what I've been doing now except when someone brings up the wrong or the right subject.)
(I hope I answered the question correctly and completely.)

And in case it wasn't clear from the code, the trigger script spawns wolves, the wolves receive Move orders making the wolves attack the players units - so basically zombie wolves. It was added to make the map less boring. The wolves are Domestic because currently only Domestic animals can follow move orders, which is explcitily code added to make it impossible, and this code had originally been removed because the perception was not shared that it should be impossible to have one kind of animal be reasonable to control and another animal not be reasonable to control, which is what D1960 addressed. I didn't read the 6000+ lines of UnitAI and not the relevant trac tickets relating to that, so I can't speak well about that.

(Also the fact that the non-domestic animals react with an animation reset but then ignore the move order is a behavior that is not present in alpha23 but in svn, so there's an unrelated regression discovered while complaining, making this the fourth way how I am constructive in triple A RTS context, or unconstructive in an easy-contribution process at intentional loss of quality.)

And in case it wasn't clear from the code, the trigger script spawns wolves, the wolves receive Move orders making the wolves attack the players units - so basically zombie wolves. It was added to make the map less boring. The wolves are Domestic because currently only Domestic animals can follow move orders […]

For me with this change the (now non-domestic) wolves move towards and attack player units after spawning the same way as they did before. What am I missing here?

elexis added a comment.Jun 1 2020, 1:50 PM

And in case it wasn't clear from the code, the trigger script spawns wolves, the wolves receive Move orders making the wolves attack the players units - so basically zombie wolves. It was added to make the map less boring. The wolves are Domestic because currently only Domestic animals can follow move orders […]

For me with this change the (now non-domestic) wolves move towards and attack player units after spawning the same way as they did before. What am I missing here?

I was wrong in saying that the units receive walk orders, because they only receive attack without move orders.
It was intended to have them use attack-walk (so that they would not pass other enemy units on their way), but it wasn't supported and then removed before the first commit (see https://code.wildfiregames.com/D156#6018).
The later maps danubius and jebel barkal use Patrol orders, which also fixes the issue that unkilled wolves stack up at enemy bases while leaving other parts of the map unattended.
Non-domestic animals do follow attack orders but not walk / attack-walk orders (both in a23b and r23724). Even in alpha 22 release which I checked out and patched.

Depending on what unitAI design people chose, it would be a bug or a feature that non-domestic units follow player sent attack/move/any order.
I would guess they follow attack orders in order to attack player owned units that may attack them. If they were meant to be to ignore player commands, they should ignore attack commands that enter through the Commands.js orderqueue.
The stop command also passes through to the non-domestic animals (a22).
Depending on what design is chosen for this map, the orders should be attack-walk orders. (And depending on what design is chosen for the map it may be deleted and moved to a mod, or whatever)

So there are different possible paths. The current patch would not break the existing attack orders, but it would make the attack-move orders impossible without D1960 or the some ticket (and it's also perhaps within the gamedesign to delete the map in favor of realism and historical accuracy. It could be hosted somewhere else.) The other option is to also prohibit the stop, attack and whatever else player-issued commands for non-domestic animals in UnitAI.js or Commands.js and keep those wolves domestic; or to replace them with non-animals (which then means they won't roam anymore. that was also one of the previously discussed solutions to a problem which already seems wrong). I don't want to dedicate to UnitAI plans, so I won't make a decision, I just wanted to mention the software design discussions that went into this weird template so that whomever wants to work on this can make some educated decision.

Nescio removed a subscriber: Nescio.Jun 1 2020, 5:34 PM
elexis added a comment.Jun 8 2020, 6:52 PM

Implications applicable to the patch depending on indeterminate values:

  • If the map is not deemed historically / realistic / on topic enough for the game, it should be deleted and could be offered in a map pack mod.
  • If the map is deemed compatible with public mod, then the unit should either patrol or not patrol.
  • If the unit does patrol, then it will never enter ROAMING state and can become a non-animal / INDIVIDUAL unit (like the briton dog), still requiring an exotic template but a different one that is only used by this map.
  • If the unit does not patrol, then the wolves will keep stacking in some parts of the map if a player hides his last units and remain sitting there for the rest of the game after that players defeat instead of cycling the reachable map parts.
  • If the unit shall not patrol, then the approach of this iteration of the patch is correct or incorrect depending on the design decision whether non-domestic animals should be able to follow attack commands issued by players.
  • If the unit shall not patrol and if the user shall be able to send attack orders to non-domestic animals, then the violent stance could probably be removed as well since it doesn't change much, thereby making the template obsolete.
  • If the unit shall not patrol and if the user shall be able to send attack orders to non-domestic animals, then the violent stance could probably be removed as well since it doesn't change much, thereby making the template obsolete.

User or trigger? D1960 would allow triggers to control every entity and still restricts tasking non-domestic animals by players. (Obviously, I vote for this.)