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

Regeneration Modifier Rework #1619

Merged
merged 14 commits into from
Jun 30, 2022

Conversation

KheirFerrum
Copy link
Collaborator

@KheirFerrum KheirFerrum commented Jun 12, 2022

Summary

SUMMARY: Infrastructure "Change regeneration modifiers into a JsonObject instead of an array."

Purpose of change

Suggested by @Coolthulhu to allow for easier read/write and to add scaling modifiers for effect intensities.

Describe the solution

Regeneration modifier in json converted to object with items Effect, base_mod and optional scaling_mod.

Temporarily adds a print message proceeding regeneration in order to know how much is actually being regenerated. Once this has been tested properly it will be removed before merge.

Describe alternatives you've considered

  • Just extend the array to contain 3 variables instead of 2.
    Considered to not be particularly user friendly for reading/writing.

Testing

  • Test that debug monster when burned has modifier adjusted accordingly.
  • Test that debug monster when corroded has modifier adjusted accordingly.

Additional context

@KheirFerrum KheirFerrum changed the title Rework Start Regeneration Modifier Rework Jun 12, 2022
@KheirFerrum KheirFerrum force-pushed the Regen-Modifier-Rework branch 2 times, most recently from 47f47ed to e001865 Compare June 13, 2022 05:53
@KheirFerrum KheirFerrum force-pushed the Regen-Modifier-Rework branch from d75c58b to d0c079a Compare June 13, 2022 06:20
src/mtype.h Outdated
@@ -200,6 +200,14 @@ struct mon_effect_data {
chance( nchance ), permanent( perm ) {}
};

struct regen_modifier {
const float base_modifier = 0.00;
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: const fields rarely useful. It is more important for variables and function arguments.

Copy link
Member

Choose a reason for hiding this comment

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

Still helps with readability IMO.

@KheirFerrum KheirFerrum marked this pull request as ready for review June 13, 2022 11:59
@KheirFerrum KheirFerrum marked this pull request as draft June 13, 2022 12:00
@KheirFerrum KheirFerrum marked this pull request as ready for review June 13, 2022 12:00
@KheirFerrum KheirFerrum force-pushed the Regen-Modifier-Rework branch from e63e4bb to 8ae5d63 Compare June 18, 2022 05:04
@Coolthulhu Coolthulhu self-assigned this Jun 19, 2022
float amount = inner.get_float( 1 );
regeneration_modifiers.emplace( effect, amount );
float base_mod = inner.get_float( "base_mod" );
float scaling_mod = ( inner.has_float( "scaling_mod" ) ) ? inner.get_float( "scaling_mod" ) : 0 ;
Copy link
Member

Choose a reason for hiding this comment

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

get_float has an overload with a fallback, used like this:

inner.get_float( "x", 0.0f );

@KheirFerrum KheirFerrum force-pushed the Regen-Modifier-Rework branch from 529415f to 09af49c Compare June 20, 2022 23:55
Update
@KheirFerrum KheirFerrum force-pushed the Regen-Modifier-Rework branch 2 times, most recently from 81ceb2e to 7721d7a Compare June 21, 2022 00:46
@KheirFerrum KheirFerrum force-pushed the Regen-Modifier-Rework branch from 7721d7a to d75060e Compare June 21, 2022 01:41
@Coolthulhu
Copy link
Member

I don't get why is Mac version of Clang complaining. Try adding an explicit operator= maybe?

Also, a user on discord had heal msgs being printed for unseen critters. I checked the current version and it looks like the heal msg is printed regardless of whether you can see the monster or not. Could bundle a fix for this with this PR, since it should be just one extra && get_player_character().sees(mon) check in an if.

@KheirFerrum
Copy link
Collaborator Author

I don't get why is Mac version of Clang complaining. Try adding an explicit operator= maybe?

Also, a user on discord had heal msgs being printed for unseen critters. I checked the current version and it looks like the heal msg is printed regardless of whether you can see the monster or not. Could bundle a fix for this with this PR, since it should be just one extra && get_player_character().sees(mon) check in an if.

I'm not sure how to do the operator=. I'll try to fix the heal message right now.

@Coolthulhu Coolthulhu merged commit ed85b51 into cataclysmbnteam:upload Jun 30, 2022
@KheirFerrum KheirFerrum deleted the Regen-Modifier-Rework branch July 4, 2022 10:10
joveeater pushed a commit to joveeater/Cataclysm-BN that referenced this pull request Jul 10, 2022
* Rework Start

* Update numbers to function right

* Styling

* Styling json

* Documentation/fixes

* Update monster.cpp

* Fixes

* Bugfix

* Trying out new things.

* Overload

Update

* Pls Work

* Update regen messages to only when seen.

* Const Remove

* That ternary operator shouldn't be needed

Co-authored-by: Coolthulhu <Coolthulhu@gmail.com>
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.

3 participants