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

[cpp/lua] Luaeffect improvements #5743

Closed
wants to merge 2 commits into from

Conversation

MowFord
Copy link
Contributor

@MowFord MowFord commented May 15, 2024

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?

Meta:

  • gives CLuaStatusEffect a pointer to track the owner of a status effect
  • sets this pointer when using entity:getStatusEffect( or entity:getStatusEffects(
  • Moves CStatusEffectContainer::RemoveStatusEffect(*pointer) function to public so it can be used by the new lua binding

Simplifies the item lua logic for #5726 for deleting effects by being able to do it directly via the object

Also, fixes a small oversight with existing effect:addMod logic in that it only gives the entity mod values if it was used within onEffectGain (and cleans up the few instances in the code that worked around this limitation, found via command below):

/mnt/c/VSCode/LSBserver/scripts$ grep :addMod -R | grep -v ^effects | grep -v pet:addMod | grep -v target:addMod | grep -v mob:addMod | grep -v player:addMod
battlefields/Apollyon/nw_apollyon.lua:    boss:addMod(xi.mod.ATTP, 100)
battlefields/Apollyon/nw_apollyon.lua:    boss:addMod(xi.mod.ACC, 50)
battlefields/Temenos/central_temenos_3rd_floor.lua:            boss:addMod(bonusMod, amount)
globals/job_utils/dragoon.lua:            wyvern:addMod(xi.mod.ACC, 6 * numLevelUps)
globals/job_utils/dragoon.lua:            wyvern:addMod(xi.mod.HPP, 6 * numLevelUps)
globals/job_utils/dragoon.lua:            wyvern:addMod(xi.mod.ATTP, 5 * numLevelUps)
globals/job_utils/geomancer.lua:    luopan:addMod(xi.mod.REGEN_DOWN, math.floor(luopan:getMainLvl() / 4) - bolsterValue)
globals/job_utils/geomancer.lua:    luopan:addMod(xi.mod.DMG, -5000)
globals/job_utils/rune_fencer.lua:            newEffect:addMod(SDT, power)
globals/job_utils/rune_fencer.lua:            newEffect:addMod(SDT, power)
globals/job_utils/rune_fencer.lua:    effect:addMod(xi.mod.INQUARTATA, effect:getPower())
globals/job_utils/rune_fencer.lua:        effect:addMod(xi.mod.PARRY_SPIKES, spikesType)
globals/job_utils/rune_fencer.lua:        effect:addMod(xi.mod.PARRY_SPIKES_DMG, effect:getSubPower())
globals/job_utils/rune_fencer.lua:        effect:addMod(resistMod, power)
globals/weaponskills.lua:        attacker:addMod(xi.mod.ACC, 40 + flourishEffect:getSubPower() * 2)
globals/weaponskills.lua:        attacker:addMod(xi.mod.ATTP, 25 + flourishEffect:getSubPower())
zones/AlTaieu/mobs/Jailer_of_Prudence.lua:        secondPrudence:addMod(xi.mod.ATTP, 100)
zones/AlTaieu/mobs/Jailer_of_Prudence.lua:        firstPrudence:addMod(xi.mod.ATTP, 100)
zones/Horlais_Peak/mobs/Dragonian_Berzerker.lua:                    mobArg:addMod(xi.mod.ATT, 200)
zones/Horlais_Peak/mobs/Dragonian_Minstrel.lua:                    mobArg:addMod(xi.mod.ATT, 200)
zones/La_Vaule_[S]/mobs/Draketrader_Zlodgodd.lua:            mobArg:addMod(xi.mod.ATT, 100)
zones/Nyzul_Isle/mobs/Raubahn.lua:                    mobArg:addMod(xi.mod.UDMGMAGIC, -10000)
zones/Nyzul_Isle/mobs/Raubahn.lua:                    mobArg:addMod(xi.mod.UDMGRANGE, -10000)

Steps to test these changes

Execute !exec player:getStatusEffect(xi.effect.REGEN) or !exec player:getStatusEffects() and see it still works as it did before

see that this bit of lua removes all effects from a player:

itemObject.onItemUnequip = function(target, item)
    for _, itemEffect in pairs(target:getStatusEffects()) do
        print(itemEffect)
        itemEffect:delStatusEffect()
    end
end

Testing the addMod extension:

  • give yourself any effect, then stack mods on it and see the mod is on your char then removed with effect goes away
    • !addeffect regen
    • !exec player:getStatusEffect(xi.effect.REGEN):addMod(xi.mod.STR, 5)
    • see that you have +5 str in equipment menu
    • cancel the effect
    • see that the +5 str goes away

@claywar claywar added the core Gives visibility for reviewers for impacted areas involving cpp label May 15, 2024
@MowFord MowFord force-pushed the luaeffect_improvements branch from 7517f67 to 4644677 Compare May 15, 2024 20:01
@zach2good
Copy link
Contributor

(I'm at a conference, so this is only a fly-by)

A Status Effect can't exist outside of being inserted into an entity's status effect container, so why not pass in the entity when the status effect is constructed, so it's always available - instead of using getters/setters and having an incomplete state?

@MowFord
Copy link
Contributor Author

MowFord commented May 15, 2024

(I'm at a conference, so this is only a fly-by)

A Status Effect can't exist outside of being inserted into an entity's status effect container, so why not pass in the entity when the status effect is constructed, so it's always available - instead of using getters/setters and having an incomplete state?

Effects actually get created before being inserted and onEffectGain is always called before it's added to a container. This means that most instances of using a cluastatuseffect will have a null entity

@MowFord MowFord force-pushed the luaeffect_improvements branch from 4644677 to 6cfd909 Compare May 15, 2024 20:55
@MowFord MowFord mentioned this pull request May 15, 2024
4 tasks
@MowFord MowFord force-pushed the luaeffect_improvements branch from 6cfd909 to 1073462 Compare May 16, 2024 22:15
This allows removing the effect via a direct binding
@MowFord MowFord force-pushed the luaeffect_improvements branch from 1073462 to b2d8c35 Compare May 22, 2024 05:15
@MowFord MowFord force-pushed the luaeffect_improvements branch from b2d8c35 to 704908e Compare May 22, 2024 05:16
@MowFord
Copy link
Contributor Author

MowFord commented May 22, 2024

Rebased and updated the new Rayke addMod call to not add extra mod to the target

@MowFord MowFord closed this May 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Gives visibility for reviewers for impacted areas involving cpp
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants