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

Missing class-specific features (class_features_data) #21

Merged
merged 14 commits into from
Aug 11, 2023
Merged

Missing class-specific features (class_features_data) #21

merged 14 commits into from
Aug 11, 2023

Conversation

gergo-szabo
Copy link
Contributor

Discussed in #19

Not discussed in #19:

  • get_class_features_data function has two parameter class_name and level. If multi class characters will be possible then this function will only need a wrapper which gets class levels and concats the results.
  • During leveling a simple call for get_class_features_data is not enough. Counter types shouldn't be reseted!

There is a huge amount of counter type mechanics. I double checked everything but it is possible I have missed something.
I created a simple test but it doesn't cover every edge case.

Closes #19

@gergo-szabo
Copy link
Contributor Author

Sorry about the spam, long day... I will finish it on Friday

dnd_character/character.py Outdated Show resolved Hide resolved
@tassaron
Copy link
Owner

tassaron commented Aug 8, 2023

Thanks so much. No worries about the 'spam', we can squash some commits down later. I appreciate your work

@gergo-szabo
Copy link
Contributor Author

Character init has a class_features_data parameter. It is needed for serialization but the init doesn't use it (class_features_data variable has a fix value until level or class changes).

test_class_features_data.py tests for:

  • no class -> None
  • no level -> default level is 1, get the correct data
  • level correct data for 9th level barbarian
  • ability score dependent class feature in case of a bard (inspiration count)
  • "days_since" and "avalable" type of features in case of a wizard
    There are other mechanisms but those are fairly simple.

@tassaron
Copy link
Owner

Since class_features_data is something internally managed by the Character class, I put the underscore signifying that it is "private". This prevents it from being serialized so we don't need it for init

I didn't put a setter method so that might be needed if the user is expected to manually modify the available values... maybe we could do some validation. But this is fine for now, I think.

I am curious why "7 day cooldown" = 999 though? What is the 999?

@gergo-szabo
Copy link
Contributor Author

I am not sure if I found every ability with cooldown. The largest cooldown I would expect is 1 year. So I choose 999 day to signal all abilities are available.
We could count the days until ability available if you want to. But then we need some logic for keeping it >= 0.

@tassaron
Copy link
Owner

Okay. Your comments of '1 day' and '7 days' are referring to the cooldown that could be implemented. The 999 is just a suitably large number that the ability wouldn't be cooled down after reset. I think I understand.

I will change the comments a bit and merge shortly. Thank you :)

@tassaron
Copy link
Owner

Sorry, I got busy yesterday. I'll work on this more tonight/tomorrow (probably tonight).

I moved your new methods into a separate file as I want to start organizing the character class and splitting it up more.

I started writing some tests but one is failing. I'll look at it tonight.

@tassaron tassaron merged commit 6458e5e into tassaron:main Aug 11, 2023
1 check passed
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.

Missing class-specific features
2 participants