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

Husks don't show owner's name in playtest-20180102 #14715

Closed
gdavegdave opened this Issue Jan 12, 2018 · 6 comments

Comments

Projects
None yet
4 participants
@gdavegdave
Contributor

gdavegdave commented Jan 12, 2018

Husks used to show the name of the player who owned the vehicle. That was useful e.g. for assessing how a battle had gone whilst speccing. In playtest-20180102 (in TD, at least), only the vehicle type is displayed when a husk is hovered. Can we have the owner's name back too?

@MustaphaTR

This comment has been minimized.

Show comment
Hide comment
@MustaphaTR

MustaphaTR Jan 12, 2018

Member

Related PR: #13276

Husks are no longer owned by their previous owner, but player "Neutral" (which has NonCombatant that hides the owner row) with EffectiveOwner of old owner to show remap correctly. But looks like it doesn't update the owner row in the way Disguise trait does. I removed NonCombatant from "Neutral" on a map but it still shows Neutral owner row.

Member

MustaphaTR commented Jan 12, 2018

Related PR: #13276

Husks are no longer owned by their previous owner, but player "Neutral" (which has NonCombatant that hides the owner row) with EffectiveOwner of old owner to show remap correctly. But looks like it doesn't update the owner row in the way Disguise trait does. I removed NonCombatant from "Neutral" on a map but it still shows Neutral owner row.

@abcdefg30 abcdefg30 added the Bug label Jan 12, 2018

@pchote pchote added Regression and removed Bug labels Jan 12, 2018

@pchote pchote added this to the Next release milestone Jan 12, 2018

@pchote

This comment has been minimized.

Show comment
Hide comment
@pchote

pchote Jan 12, 2018

Member

The code that decides whether or not to show the owner row probably doesn't check the EffectiveOwner.

Member

pchote commented Jan 12, 2018

The code that decides whether or not to show the owner row probably doesn't check the EffectiveOwner.

@MustaphaTR

This comment has been minimized.

Show comment
Hide comment
@MustaphaTR

MustaphaTR Jan 12, 2018

Member

I still get neutral owner row, rather than mine on husks, even if i remove NonCombatant from Neutral. Even when visibility cheat is disabled. So i don't think it is the code to show or hide the owner row. I was going to test with spy too, but it doesn't look like i can make myself non-combatant, so i need to write some lua to test.

resim

Member

MustaphaTR commented Jan 12, 2018

I still get neutral owner row, rather than mine on husks, even if i remove NonCombatant from Neutral. Even when visibility cheat is disabled. So i don't think it is the code to show or hide the owner row. I was going to test with spy too, but it doesn't look like i can make myself non-combatant, so i need to write some lua to test.

resim

@MustaphaTR

This comment has been minimized.

Show comment
Hide comment
@MustaphaTR

MustaphaTR Jan 12, 2018

Member

Spies work fine, there is something wrong with Husks.

Visibility Cheat disabled:
resim
Visibility Cheat enabled:
resim

Member

MustaphaTR commented Jan 12, 2018

Spies work fine, there is something wrong with Husks.

Visibility Cheat disabled:
resim
Visibility Cheat enabled:
resim

@pchote

This comment has been minimized.

Show comment
Hide comment
@pchote

pchote Jan 12, 2018

Member

Again, the code that displays the tooltip doesn't check for the EffectiveOwner. Instead, spies special case that logic as part of their disguise tooltip, which is wrong because the effective owner handling should be logically distinct from the change in effective actor.

Member

pchote commented Jan 12, 2018

Again, the code that displays the tooltip doesn't check for the EffectiveOwner. Instead, spies special case that logic as part of their disguise tooltip, which is wrong because the effective owner handling should be logically distinct from the change in effective actor.

@pchote

This comment has been minimized.

Show comment
Hide comment
@pchote

pchote Jan 24, 2018

Member

If nobody volunteers a fix for this then IMO it should be promoted to a feature (and probably properly toggled in the yaml) and dropped from the milestone.

Member

pchote commented Jan 24, 2018

If nobody volunteers a fix for this then IMO it should be promoted to a feature (and probably properly toggled in the yaml) and dropped from the milestone.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment