Skip to content

[Fix] Light shot#10069

Merged
WinterSolstice8 merged 1 commit into
LandSandBoat:basefrom
Xaver-DaRed:shot
May 22, 2026
Merged

[Fix] Light shot#10069
WinterSolstice8 merged 1 commit into
LandSandBoat:basefrom
Xaver-DaRed:shot

Conversation

@Xaver-DaRed
Copy link
Copy Markdown
Contributor

I affirm:

  • I understand that if I do not agree to the following points by completing the checkboxes my PR will be ignored.
  • I understand I should leave resolving conversations to the LandSandBoat team so that reviewers won't miss what was said.
  • I have read and understood the Contributing Guide and the Code of Conduct.
  • I have tested my code and the things my code has changed since the last commit in the PR and will test after any later commits.

What does this pull request do?

  • Limits amount of boosts Dia can get
  • Corrects boosted values.

Steps to test these changes

Test it.

target:delStatusEffectSilent(effectId)
target:addStatusEffect(effectId, { power = power, duration = duration, origin = player, tick = tick, subType = subId, subPower = subpower, tier = tier })
diaPower = diaPower + 1
diaSubpower = diaSubpower + 28 / 1024
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

subPower in addStatusEffect is an integer
so I'm not sure if this is doing what you intend it to do

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Should I just change the data type to a float in there?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

in CStatusEffect:

    uint16 GetPower() const;
    uint16 GetSubPower() const;

it would basically require an overhaul of that class to support floats directly. Maybe we could use a union and have a fetch power/subpower for both float/uint...

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The quickest fix to get this mostly working is to use this instead

diaSubpower = diaSubpower + math.floor(28 / 1024) * 100

this results in an extra -2 DEFP in the dia effect script

effectObject.onEffectGain = function(target, effect)
    effect:addMod(xi.mod.REGEN_DOWN, effect:getPower())
    effect:addMod(xi.mod.DEFP, -effect:getSubPower())
end

(28/1024)*100 = 2.734375, so .73~ish percent DEF reduction is lost. we should probably make a comment to add a different DEFP/ATTP/RATTP mod or increase the resolution of ATTP/DEFP/RATTP to account for SE shenanigans.

If it were me, I'd add a second mod for each of them, since the mods are only effective with one access each in:

uint16 CBattleEntity::RATT(uint16 bonusAtt)
uint16 CBattleEntity::ATT(SLOTTYPE slot)
uint16 CBattleEntity::DEF()
And the boost test in xi_test (though it is simply checking the value is zero/nonzero for ATTP.)

The reasoning behind a second mod is to avoid some potential fallout from mods getting converted. Not to mention that sometimes SE actually does use real percents, so it would cover both uses by SE.

Comment thread scripts/actions/abilities/light_shot.lua
@WinterSolstice8
Copy link
Copy Markdown
Contributor

Looks like the spec for setOriginId was never added

(originid is lowercase in CLuaStatusEffect, but it probably doesn't need to match.(

diff --git a/scripts/specs/core/CStatusEffect.lua b/scripts/specs/core/CStatusEffect.lua
index 027a2af136..b5c1edd23a 100644
--- a/scripts/specs/core/CStatusEffect.lua
+++ b/scripts/specs/core/CStatusEffect.lua
@@ -149,3 +149,8 @@ end
 ---@return integer
 function CStatusEffect:getOriginID()
 end
+
+---@param originid integer
+---@return nil
+function CStatusEffect:setOriginID(originid)
+end

@Xaver-DaRed
Copy link
Copy Markdown
Contributor Author

I will tackle the modifiers problem in a different PR. For now I think this is a good base to tackle the different shots (I assume they all need a similar treatment)

@WinterSolstice8 WinterSolstice8 merged commit dc3011e into LandSandBoat:base May 22, 2026
10 checks passed
@Xaver-DaRed Xaver-DaRed deleted the shot branch May 22, 2026 12:55
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.

2 participants