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

Combat turn V3 #245

Merged
merged 4 commits into from
Mar 11, 2025
Merged

Combat turn V3 #245

merged 4 commits into from
Mar 11, 2025

Conversation

Azurelol
Copy link
Collaborator

@Azurelol Azurelol commented Mar 8, 2025

  • Standardizes the combat turn button across the combat hud and tracker
  • Fixes taking turns out of order
  • Show info for other combatants when someone's taking a turn
  • Add item event

image

@Azurelol Azurelol changed the base branch from main to dev March 8, 2025 19:29
@Azurelol Azurelol force-pushed the fix/combat_turn_again branch 2 times, most recently from a9fea4c to fda70e8 Compare March 8, 2025 21:02
@Azurelol Azurelol marked this pull request as ready for review March 8, 2025 21:03
@Azurelol Azurelol force-pushed the fix/combat_turn_again branch from fda70e8 to b3b795c Compare March 8, 2025 21:05
@Azurelol
Copy link
Collaborator Author

Azurelol commented Mar 8, 2025

Don't be afraid! The line changes come from fixing the line endings for the package files.

@Azurelol Azurelol linked an issue Mar 8, 2025 that may be closed by this pull request
@Azurelol Azurelol force-pushed the fix/combat_turn_again branch 2 times, most recently from 588e64e to b4f19a3 Compare March 9, 2025 12:02
* @param {FUCombat} combat
* @return {Object.<"friendly"|"neutral"|"hostile", {}[]>}
*/
getFactions() {

Choose a reason for hiding this comment

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

The original version of this method was not cleaned up combat-tracker.mjs:

async getFactions(turns, combat) {

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ack. Thank you for pointing it out.

data.combatant = this.combatant;
// ID : Turns Left
data.turnsLeft = this.countTurnsLeft();
data.factions = this.getFactions();
Copy link

@Element-Re Element-Re Mar 9, 2025

Choose a reason for hiding this comment

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

Might need to pass data.turns into this (see below)

Copy link
Collaborator Author

@Azurelol Azurelol Mar 10, 2025

Choose a reason for hiding this comment

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

Rather, it was changed so that factions only gets assigned if not already present. Since the combat hud will use it as well.

* @return {Object.<"friendly"|"neutral"|"hostile", {}[]>}
*/
getFactions() {
return this.combatants.reduce(
Copy link

@Element-Re Element-Re Mar 9, 2025

Choose a reason for hiding this comment

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

This is the crux of the issue with with the missing animated token thumbnails and other elements like effects or resources. The previous version of getFactions had data.turns passed in from populateData, and it was this collection of turn data that was iterated over in the combat tracker handlebars template. This now uses the combatants collection on the combat, which is missing information compared to the turn data array previously passed in (such as a dynamic url based on either the token's image itself, or a base64 png display url generated from the token's sprite texture instead.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This was correct, as the combat tracker needs different data.

@Azurelol Azurelol force-pushed the fix/combat_turn_again branch 4 times, most recently from 2110832 to 43220ab Compare March 10, 2025 19:50
@Azurelol
Copy link
Collaborator Author

Azurelol commented Mar 10, 2025

After the rebase + fixup for the damned files, the turn button broke. Investigating...

Fixed.

@Azurelol Azurelol force-pushed the fix/combat_turn_again branch from 43220ab to faa449a Compare March 10, 2025 20:24
…at is when data is retrieved

Whether combat has started depends on whether round > 0
@Azurelol Azurelol requested a review from Element-Re March 10, 2025 21:18
agg[combatant.id] = combatant.totalTurns;
return agg;
}, {});
// We assign it afterwards, since factions will be assigned if empty
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure I understand this. What do we mean by afterwards? The logic is not deferred here right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Whops, forgot to remove this comment. (Old implementation)

@Azurelol Azurelol merged commit 11022d0 into dev Mar 11, 2025
@Azurelol Azurelol deleted the fix/combat_turn_again branch March 11, 2025 16:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Combat HUD/tracker issues on 2.4.9-alpha.5
3 participants