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

Add support for improvised effect calculation on inline damage and resource gain/loss #180

Merged

Conversation

Azurelol
Copy link
Collaborator

@Azurelol Azurelol commented Dec 6, 2024

Adds support for using improvised effect into the following inline command ,where improvised effect:

$effect: Minor|Heavy|Massive

  • @DMG[$number $type], @DMG[$effect $type]
  • @GAIN[$number $type] @GAIN[$effect $type]
  • @LOSS[$number $type], @LOSS[$effect $type]

This allows having spells/skills scale to the level of the source within the following ranges:

  • [5,19]
  • [20,39]
  • [40,INF]

The amount calculated from an improvised effect is a simple mapping between the level range and the effect "potency" where the level is calculated based on the owner of the item. This works equally for spells, skills and active effects. (When an active effect is dropped on an actor, that becomes its source)

Example:
image

/**
* Tier: Effect [Minor,Heavy,Massive]
*/
const amountPerTier = [
Copy link
Collaborator

Choose a reason for hiding this comment

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

Isn't directly clear how this data structure is organized at first glance. Comment could be interpreted as the first index being minor/heavy/massive.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good point. I will rewrite this comment for improved clarity.

function calculateAmountFromContext(dataset, source, targets) {
const effect = dataset.effect;
if (effect === undefined) {
return Number(dataset.amount);
Copy link
Collaborator

@KSops KSops Dec 6, 2024

Choose a reason for hiding this comment

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

If dataset.amount is undefined, it looks like this produces NaN.
The JSDoc indicates the return should be null on failure.

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. Thanks, will fix.

}

const amount = ImprovisedEffect.calculateAmount(level, effect);
console.info(`Calculated amount for level ${level}, effect ${effect} = ${amount}`);
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: console.debug might help reduce console clutter

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Whoops. Sorry, did not mean to commit this here. I will remove.

level = source.system.level.value;
} else {
if (targets.length > 0) {
level = targets.reduce((a, b) => b.system.level.value > a.system.level.value);
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is intended to return the highest level of targets? Couldnt get the reduce to work in a minimal example

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, to take the highest level.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think something closer to this might achieve the max level (stolen from AI):

let maxValue = targets.reduce((max, target) => {
    return Math.max(max, target.system.level.value);
}, -Infinity); // Start with negative infinity to ensure any number is higher

Copy link
Collaborator Author

@Azurelol Azurelol Dec 11, 2024

Choose a reason for hiding this comment

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

Done. It worked correctly as well.

@Azurelol
Copy link
Collaborator Author

Azurelol commented Dec 8, 2024

Sorry for the late follow-up; will address your feedback now.

@Azurelol
Copy link
Collaborator Author

Azurelol commented Dec 8, 2024

Thank for your feedback, @KSops !

@Azurelol Azurelol requested a review from KSops December 8, 2024 19:35
@KSops
Copy link
Collaborator

KSops commented Dec 8, 2024

Thanks for the great addition!

@spyrella spyrella merged commit bb681eb into League-of-Fabulous-Developers:dev Dec 11, 2024
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