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

Trackers #57

Merged
merged 13 commits into from
Nov 27, 2023
Merged

Trackers #57

merged 13 commits into from
Nov 27, 2023

Conversation

voidrender
Copy link
Member

Describe your changes

  • Updated conversion process to convert DDB exp and HP to the first and second trackers on the created character
  • Updated types, tests, and removed deprecated fields

A few months ago, I implemented HP and XP trackers on the Alchemy side of the import process, but in fixing a bug this week related to importing from JSON, I realized that it would be advantageous for this conversion to happen within ddb2alchemy instead. There are a few benefits:

  • Simpler import process on the Alchemy side since we don't need a separate code path to handle trackers for JSON imports vs. DDB imports.
  • More flexible ddb2alchemy API for future trackers (things like temp HP, inspiration, ki points, sorcery points, etc.).

Demo

I pointed my local build of the Alchemy app to this branch to test the import process with the below D&D Beyond character link.

2023-11-23.at.21.29.53.-.White.Stork.mp4

D&D Beyond character link

https://www.dndbeyond.com/characters/22233192/BKPCYD

Pre-review checklist

  • I have added a description of my changes
  • I have formatted my code with yarn format
  • I have added a link to a D&D Beyond character that can be used to test these changes
  • I have read the contributing guidelines
  • I agree to the code of conduct

Copy link
Collaborator

@thatbudakguy thatbudakguy left a comment

Choose a reason for hiding this comment

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

thanks for sparing some time for ddb2alchemy in the thick of all the other v1 work! excited to use this in the future for things like ki points, etc.

src/convert.ts Outdated
name: 'XP',
category: 'experience',
color: 'Yellow',
max: 355000,
Copy link
Collaborator

Choose a reason for hiding this comment

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

doesn't necessarily need to hold up this PR, but I imagine the most common way people use the XP tracker is to track XP until their next level (rather than theoretical max XP at lv20, or something). seems like all we'd need to do to support that is to have a static map between levels and XP counts, and then use the character's level to put the correct total here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point! And yeah, we actually do this for characters created in Alchemy, too. I could yoink that function from our code.

Copy link
Member Author

Choose a reason for hiding this comment

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

Alright, I went head and pulled in those functions from the Alchemy 5e implementation and updated the way the XP tracker is created so that it tracks XP until next level now.

Instead of 0 / 355000, set the XP tracker's current `value` to the
amount of experience earned within the current level and the `max` to
the experience required to reach the next level.
@voidrender
Copy link
Member Author

Whoops, I just spotted an error, fixing...

Instead of adjusting the value relative to the current level (which
would look nice on the bar), this instead shows the current total
experience as the value, and the next level as the max. This is closer
to what D&D Beyond shows, although it has a nicer bar that shows the
progress to the next level. We don't really have a way to do that solely
with a tracker at the moment, though.
@voidrender
Copy link
Member Author

Okay, now it's working correctly and in a way that mirrors what you see in D&D Beyond pretty closely. The only thing that's a little weird is that the tracker bar doesn't show relative % towards the next level, but I don't think there's much we can do about that right now without adjusting the value/max for the relative distance to the next level, which would look a bit odd numerically.

2023-11-26.at.20.51.20.-.Gold.Otter.mp4

@voidrender voidrender merged commit d75a707 into main Nov 27, 2023
2 checks passed
@voidrender voidrender deleted the trackers branch November 27, 2023 07:29
@voidrender
Copy link
Member Author

Alchemy is on this version now, as of about 2 minutes ago. 🚀

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.

None yet

2 participants