Page MenuHomeWildfire Games

Headless Hoplite
ClosedPublic

Authored by Stan on Jul 22 2017, 10:41 PM.

Details

Reviewers
elexis
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Restricted Owners Package(Owns No Changed Paths)
Commits
rP19948: Fix detached heads of Spartan elite spearmen.
Trac Tickets
#4686
Summary

See ticket.

Test Plan

Check in the actor viewer than the gathering and carrying anims are correct, and that using the commands.txt you can't reproduce the bug.

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

Stan created this revision.Jul 22 2017, 10:41 PM
Vulcan added a subscriber: Vulcan.Jul 22 2017, 11:28 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!
Checking XML files...

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

elexis added a subscriber: elexis.Jul 23 2017, 12:46 AM

The other hoplites are not affected? Maybe we could do a grep across all unit types where a head is attached to the helmet.
Can we do something about the units in gather animation not having helmets while at it, or is that more elaborate?

Stan added a comment.EditedJul 23 2017, 11:15 AM

I checked every infantry unit, that's the only one I could find for which that occurs. Every other unit has his own head when gathering.

About the other issue you mentionned, I don't know why Enrique did it that way, but he probably has a reason.

So first ask him,
Then remove the line

<prop actor="" attachpoint="helmet"/>

in the gathering files. or edit every single actor you want to keep their helmet and change the variant.
I feel like this out of the scope of the ticket as it's more a "feature" than a bug.

(Just for curiosity: Considering the fact that in aplha 21 the face, but not the entire head was shown, the change in rP19095 replaced one headless bug with another headless bug?)

Stan added a comment.Jul 23 2017, 6:11 PM

Actually the old bug was because helmets were removed when gathering, the file was correct. There was only the face not the hair on the meshes thus giving the impression half the face was missing.

In D752#29466, @Stan wrote:

There was only the face not the hair on the meshes thus giving the impression half the face was missing.

Wow, how are the chances of changing one headless-unit bug to another headless-unit bug.

When doing a grep, I only find one more occurance that has the same pattern which was also introduced by that commit https://trac.wildfiregames.com/changeset/19095/ps/trunk/binaries/data/mods/public/art/actors/units/spartans/agis.xml, does that all work as intended? (Couldn't see anything broken ingame when walking, idling, attacking or capturing.) Is the change of that commit (new head mesh?) in place correctly?

units/spartans/agis.xml:        <prop actor="props/units/heads/new/head_corinthian.xml" attachpoint="helmet"/>
Stan added a comment.EditedJul 23 2017, 7:27 PM

Replacing helmet by head would fix an hidden bug due to the fact this unit is a hero which is why he doesn't have any gathering anims. Also a graphical one because head is not in the right position.

elexis accepted this revision.Jul 28 2017, 6:59 AM
elexis edited the summary of this revision. (Show Details)

Thanks for the fix.

This revision is now accepted and ready to land.Jul 28 2017, 7:00 AM
In D752#29466, @Stan wrote:

Actually the old bug was because helmets were removed when gathering, the file was correct. There was only the face not the hair on the meshes thus giving the impression half the face was missing.

The actor attached a head mesh that can only be used with a helmet, while the actor doesn't attach a helmet. So the file was not corrrect. Correct?

In D752#29490, @Stan wrote:

Replacing helmet by head would fix an hidden bug due to the fact this unit is a hero which is why he doesn't have any gathering anims. Also a graphical one because head is not in the right position.

Then the next one touching that model should be careful to notice and fix this hidden bug and check that there are no zombies walking around after committing the new actor.

This revision was automatically updated to reflect the committed changes.
Stan added a comment.Aug 7 2017, 1:37 PM

The actor attached a head mesh that can only be used with a helmet, while the actor doesn't attach a helmet. So the file was not corrrect. Correct?

Yeah.

Then the next one touching that model should be careful to notice and fix this hidden bug and check that there are no zombies walking around after committing the new actor.

Indeed. If I'm still there by then (hopefully) I'll check.

Commit

Thanks for commiting it :)