-
-
Notifications
You must be signed in to change notification settings - Fork 81
Some NPC.HitModifiers.SourceDamage patches: #100
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
base: 1.4.4
Are you sure you want to change the base?
Conversation
Replaced globally available projectile instances of HitModifiers.SourceDamage modification with HitModifiers.TargetDamageMultiplier, so as to prevent it from being inherited in NPC.HitInfo.SourceDamage and doubled up in cases where other mods may spawn additional OnHit projectiles that inherit HitInfo.SourceDamage for accurate OnHit interactions. Recommended to move to HitModifiers.FinalDamage and redone as such or implement defensive mechanisms that don't let additional projectiles inherit SourceDamage, like globally tagging projectiles (and their children by chaining projectile tags with e.g. `source is EntitySource_Parent parent && parent.Entity is Projectile proj && proj.Calamity().WeaponSource`) in OnSpawn by checking EntitySource_ItemUse_WithAmmo. A few other less important examples were left as is, including Thanatos closed vent DR and GFB seed interaction(s?) like Profaned Guardians.
…ic (CalamityTeam#97) Fix: Correct inverted logic in AnySolidTileInSelection
* Added new PlayerUtil method to disable the default Item32 wing flap sound * Added comments to clarify WingFlap changes and usage
CalamityTeam#91) Made spacing consistent for attunements tooltips of biome blade (and variants)
…m#93) Removed the unnecessary reinitialization of ManagedTargets in ModLoad because it removed some of the rendertargets on the mod that loaded before OnModLoad
Ozzatron
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All of the changes are sound except for Frost Armor. Frost Armor's current behavior becoming multiplicative presents a significant balance concern. Remove the changes to Frost Armor and this PR is fine as-is; the team will change Frost Armor internally.
The Frost Armor tweak is not necessarily multiplicative in the way you'd imagine. I've done a small hack there to imitate the same effect its additiveness would have, as TargetDamageMultiplier is a MultipliableFloat which means it can't be simply added onto. In the meanwhile, I added a small safety check that I just noticed was lacking with my implementation. |
Ozzatron
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your counterargument is accepted. Good catch on the safety check. In truth we'll likely have to rewrite how Frost Armor works either way but I agree with you that this is good enough for now.
The merge-base changed after approval.
Replaced globally available projectile instances of HitModifiers.SourceDamage modification with HitModifiers.TargetDamageMultiplier, so as to prevent it from being inherited in NPC.HitInfo.SourceDamage and doubled up in cases where other mods may spawn additional OnHit projectiles that inherit HitInfo.SourceDamage for accurate OnHit interactions. Recommended to move to HitModifiers.FinalDamage and redone as such or implement defensive mechanisms that don't let additional projectiles inherit SourceDamage, like globally tagging projectiles (and their children by chaining projectile tags with e.g.
source is EntitySource_Parent parent && parent.Entity is Projectile proj && proj.Calamity().WeaponSource) in OnSpawn by checking EntitySource_ItemUse_WithAmmo. A few other less important examples were left as is, including Thanatos closed vent DR and GFB seed interaction(s?) like Profaned Guardians.