Opened 12 years ago

Closed 11 years ago

Last modified 11 years ago

#1260 closed defect (fixed)

[PATCH] The resource carrying animation is not displayed, or displayed when not needed

Reported by: Евгений Owned by: leper
Priority: Nice to Have Milestone: Alpha 12
Component: UI & Simulation Keywords: patch
Cc: Patch:

Description

If the worker has a 20/20 resources, his bag shows only when he goes to drop resource. In all other cases (if worker stands or goes somewhere) the bag is missing. If many workers will stand with the resources, but without the bags, then you might think that they are without resources. This is confusing. I think the bag should be displayed always, if worker have at least 1/20 resources.

Happens that a worker has a 0/20 resources (it is if when he began to extract resources, you interrupt this), in which case the bag with him if you tell him to go to the building where he can drop resource. Like on screenshot: http://i.imgur.com/s3BEZ.jpg I think in this case bag and icon of resource on the panel should not be.

Game version: Alpha 9 Ides of March. My PC: Windows 7 Ultimate, 64-bit (Service Pack 1), AMD Phenom(tm) 9850 Quad-Core Processor 2.49 GHz, GeForce? GTX 260 (driver version 295.73), (two monitors 1920x1200, 1440x900), 4 Gb RAM.

Attachments (3)

UnitAI.js.patch (1.1 KB ) - added by Harco Gijsbers 12 years ago.
Patch with a partial fix for this issue
show-carry-anim.patch (12.2 KB ) - added by Deiz 12 years ago.
show-carry-anim.2.patch (11.7 KB ) - added by Deiz 12 years ago.

Download all attachments as: .zip

Change History (18)

comment:1 by historic_bruno, 12 years ago

Component: Art & AnimationUI & Simulation

comment:2 by historic_bruno, 12 years ago

Keywords: simple added; bag removed
Summary: The worker bag is not displayed, or displayed when it do not need.The resource carrying animation is not displayed, or displayed when it do not need.

comment:3 by vts, 12 years ago

Summary: The resource carrying animation is not displayed, or displayed when it do not need.The resource carrying animation is not displayed, or displayed when not needed

by Harco Gijsbers, 12 years ago

Attachment: UnitAI.js.patch added

Patch with a partial fix for this issue

comment:4 by Harco Gijsbers, 12 years ago

Summary: The resource carrying animation is not displayed, or displayed when not needed[PATCH] The resource carrying animation is not displayed, or displayed when not needed

comment:5 by Harco Gijsbers, 12 years ago

Keywords: review added
Milestone: BacklogAlpha 11

comment:6 by Harco Gijsbers, 12 years ago

The attached patch fixes that the carrying animation is shown when an unit is walking to a resource target and is already carrying a resource of some kind.

How to test:
1) Gather some resources with an unit
2) Interrupt the gathering after the unit has at least 1 resource
3) Make the unit stand idle for a moment
4) Start gathering from a resource again
5) The appropriate carrying animation is shown (based on the resource gathered on step 1

The second part of this ticket i cannot reproduce. Maybe someone else fixed it ?

--- Looks like philip fixed it in revision 8904.

Last edited 12 years ago by Harco Gijsbers (previous) (diff)

comment:7 by Harco Gijsbers, 12 years ago

I am planning to fix that idle units who are also carrying a resource show the correct animation in the next days. It seems to me (like the newby i am) that we need a new animation for idle units which are also holding a resource.

I have already tried the "carry_<resource>" animation but that looks a bit silly. It looks like the unit is walking while it standing idle. When i call the animation just once it still looks not right.

Is it correct for me the say that we need a new animation for this ?

Last edited 12 years ago by Harco Gijsbers (previous) (diff)

comment:8 by Deiz, 12 years ago

Keywords: review removed

I'm unsure how the second part of the ticket could occur as recently as four months ago. Most of the relevant code dates to late 2010; at present it seems impossible for ResourceGatherer's lastCarriedType to be set without the unit actually carrying at least 1 of a resource.

I don't think the 'switch to other resource' case is important enough to merit special code just for it. Instead, perhaps there could be an 'animation replacement' look-up table for use by SelectAnimation, e.g.:

replaceAnimation = { "walk": "carry_" + type.generic }

It'd be set when the gatherer was leaving GATHER.GATHERING, if they're carrying anything, and whenever attempting to set a "walk" animation, the carrying-resource animation would be used instead (except perhaps when overridden, e.g. when moving to attack).

You're right that a new animation would be required, but looking at how the existing carried-resource animations work by plopping a resource prop on top of the usual 'walk' animation, I don't think showing the resource while idle would be difficult. Based on a bit of IRC discussion it's not clear that it's desirable to show carried resources while idle (as it hides weapons, etc.), so perhaps that's something to be discussed on the forums.

Also, I notice that your patch is indented with spaces; 0 A.D. uses actual tab characters.

comment:9 by Harco Gijsbers, 12 years ago

Thanks for reviewing. I'll give the animation replacement table a try.

comment:10 by Deiz, 12 years ago

Had some time this morning and looked into how the movement animations are handled. Turns out UnitAI typically triggers a special mode in CCmpVisualActor, after which the animation may be idle, walk, or run. Since UnitAI has no way of knowing when the animation changes, it was necessary to implement an override in the engine.

Attached patch replaces the 'walk' anim for any unit that has gathered resources, as long as it's not actively moving into combat (approaching to attack or chasing a target).

by Deiz, 12 years ago

Attachment: show-carry-anim.patch added

by Deiz, 12 years ago

Attachment: show-carry-anim.2.patch added

comment:11 by Kieran P, 12 years ago

Milestone: Alpha 11Alpha 12

comment:12 by Kieran P, 11 years ago

Keywords: patch review added; resource simple removed

comment:13 by leper, 11 years ago

m_CurSpeed needs to be serialized (see r12941).

comment:14 by leper, 11 years ago

Owner: set to leper
Resolution: fixed
Status: newclosed

In 12949:

Add animation override to VisualActor to fix carrying resource animations. Patch by Deiz. Fixes #1260, #1741.

in reply to:  13 comment:15 by leper, 11 years ago

Keywords: review removed

Replying to leper:

m_CurSpeed needs to be serialized (see r12941).

Actually no, as this is computed every turn.

This seemed like a sane approach and fixed this and #1741 so I went forward and committed it.

Note: See TracTickets for help on using tickets.