Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Minor] Use SetAnimOwnerHouseKind to set owner for anim #1502

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

Coronia
Copy link
Contributor

@Coronia Coronia commented Jan 20, 2025

Currently, many anims are setting their ownership with pAnim->Owner= method, which actually misses several check and set logic of Phobos (mostly related to create unit). This is another consistency fix to make all of these cases use the proper function to set anim's ownership.

@github-actions github-actions bot added the Minor Documentation is not required label Jan 20, 2025
Copy link

github-actions bot commented Jan 20, 2025

Nightly build for this pull request:

This comment is automatic and is meant to allow guests to get latest nightly builds for this pull request without registering. It is updated on every successful build.

@Starkku
Copy link
Contributor

Starkku commented Jan 20, 2025

Shield and AE anims will never actually execute MakeInfantry/CreateUnit code because they play for infinite iterations regardless of explicit LoopCount setting, so for them this is superfluous even though it would appear that shield animation had the call prior to this PR even.

@Coronia
Copy link
Contributor Author

Coronia commented Jan 21, 2025

Shield and AE anims will never actually execute MakeInfantry/CreateUnit code because they play for infinite iterations regardless of explicit LoopCount setting, so for them this is superfluous even though it would appear that shield animation had the call prior to this PR even.

The one I've added in ShieldClass is for BreakAnim, which isn't its attached looped anim, think it should be legit. Can revert this setting for AE and shield's IdleAnim if needed

@Coronia
Copy link
Contributor Author

Coronia commented Jan 21, 2025

added SetInvoker to the following cases as well

  • deploy/undeploy anims
  • promotion anims
  • shield's hit/break/idle anims
  • PassengerDeletion.Anim
  • Crit.ActiveChanceAnims
  • AE anim when Animation.UseInvokerAsOwner is set to false, which will set Invoker to the attached techno

btw, I've noticed that shield's BreakAnim is attached to the shielded object, while HitAnim doesn't. Should they follow the same scheme here?

@Coronia Coronia force-pushed the anim-create-fix branch 2 times, most recently from 37d68d7 to 2b201e1 Compare January 21, 2025 03:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Minor Documentation is not required
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants