Implement 10 Relics #267
Open
Implement 10 Relics #267
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
*initial relic setup *implement effects complete: *Cavern: Band, Eagle, Firesmith, Guard, Messenger, Thief, Wastelander *Planar: Celestial incomplete: *Cavern: Disciple *Planar: Keel To-Do: *Disciple: in onHPChange, it is assumed e.Target means source and compares to mod owner (= wearer; similar to simimpact#195) need to check whether this is correct *Keel: how to handle `applied` and pointer logic (idea from conversation on simimpact#124; once finished, can be used for Fleet impl since they are functionally almost identical)
aaron-hwang
reviewed
Jul 30, 2024
Collaborator
aaron-hwang
left a comment
aaron-hwang
left a comment
There was a problem hiding this comment.
Some minor formatting/general changes
addressed review simimpact#267 (review) * Band, Firesmith, Messenger: refactored singular `onAdd` function calls * Guard: removed `CanModifySnapshot` initialization * Firesmith, Celestial: renamed `onAfterAttack` function to clarify what it does * Wastelander: remove `float64` cast
* fixed and improved logic for `applied` and related pointers * removed unnecessary initialization of `applied` inside `keelcdmg` modifier
kdovtdc
commented
Aug 24, 2024
* KNOWN BUG: this will also buff owners equipping this 4p set even if they do not target allies with an Ult * INACCURACY: this should apply instead when OnBeforeAction is triggered
kdovtdc
commented
Sep 5, 2024
* update with new RWSD flag + `CanDispel` field * fix Celestial to correctly for CDmg and apply during `BattleStart`
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
complete:
incomplete:
Cavern: Disciple, MessengerPlanar: KeelNotes:
in onHPChange, it is assumedneedse.Targetmeans source and compares to mod owner (= wearer; similar to Implement The Unreachable Side (LC) #195) need to check whether this is correcte.Source(from AddSourcetoHPChangeevent's emit and fix impls #281)* Keel: how to handle(idea from conversation on Implemented Fleet [Relic] #124; once finished, can be used for Fleet impl since they are functionally almost identical)appliedand pointer logicOnBeforeAction, which requires the action's target list (event.ActionStartdoesn't have it) -> will be left as a bug (issue created: Add eligible targets list for certain event(s) #283 )