Skip to content

Conversation

Iridar
Copy link

@Iridar Iridar commented Jan 28, 2023

FIxes #84

Move SPARKs and other large units slightly to the right so they no longer obscure the promotion screen.

@Iridar Iridar added this to the Version 1.1 milestone Jan 28, 2023
@Iridar Iridar self-assigned this Jan 28, 2023
@Iridar Iridar added the enhancement New feature or request label Jan 28, 2023
@Iridar
Copy link
Author

Iridar commented Jan 28, 2023

{
PawnLocation.X += LargeUnitPawnOffset;
}
XComUnitPawnNativeBase(ActorPawn).SetLocationNoCollisionCheck(PawnLocation);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not 100% sure if this is the correct place to update this -- conceptually this should probably happen when actually creating the pawn so that the UIPawnMgr's internal info also has the correct info.

Also this makes PopulateData not idempotent, which seems dangerous to me -- if PopulateData is called multiple times (skimming the code, e.g. by purchasing more abilities) the Spark would move a bunch of units to +X multiple times...

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not 100% sure if this is the correct place to update this -- conceptually this should probably happen when actually creating the pawn so that the UIPawnMgr's internal info also has the correct info.

Also this makes PopulateData not idempotent, which seems dangerous to me -- if PopulateData is called multiple times (skimming the code, e.g. by purchasing more abilities) the Spark would move a bunch of units to +X multiple times...

Good points, will adjust.

@Iridar Iridar force-pushed the 84-SPARKS-too-big branch from d20dde6 to d6e104c Compare February 5, 2023 05:28
…ht so they no longer obscure the promotion screen.
@Iridar Iridar force-pushed the 84-SPARKS-too-big branch from d6e104c to 3613201 Compare February 5, 2023 05:43
@Iridar
Copy link
Author

Iridar commented Feb 5, 2023

Ran some tests, and you were right, the pawn was indeed getting slowly right-shifted into oblivion when entering and exiting the promotion screen repeatedly.

I've adjusted the code to add the offset to the location we get from the armory's placement actor instead of the pawn itself. Since the location of the placement actor remains unchanged, the pawn won't get offset from it more than once.

I've also moved the offset function call from PopulateData() into CreateSoldierPawn(), though due to Firaxis' noodle code, that function doesn't actually release the old pawn, and neither does the RequestPawn(), which is the function that's actually responsible for creating the pawn... most of the time. The underlying code is kinda awful, so there's no single good place to put the offset function call into.

I've also added some code to move the pawn back to the placement actor's location when the promotion screen is removed.

I briefly tested the new version and everything seems to be working fine.

@Iridar Iridar requested a review from robojumper February 5, 2023 05:43
@Iridar Iridar added the ready-for-review Waiting to be peer-reviewed label Feb 8, 2023
@Iridar Iridar merged commit 03627a9 into master Feb 13, 2023
@Iridar Iridar deleted the 84-SPARKS-too-big branch February 13, 2023 20:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request ready-for-review Waiting to be peer-reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SPARKs are still too big for the promotion screen
3 participants