Add More Substantial Override Paths For Physical Combat, 9/11/2024

[magicono43]

The idea behind these changes is to allow for much more of the vanilla behavior for the physical combat systems to be circumvented if a modder chooses to, the override methods in FormulaHelper being there so they can also do whatever actions they want to instead. In this instance hopefully with as little impact on other parts of the code and execution as possible, 9/11/2024.

My current desire for these is that just overriding the CalculateDamage method does not all suppression of vanilla stuff that happens regardless of what is changed in that damage calculation method, things such as sounds being played being the most notable one, and others as well.

Note: I don't know why in Github Desktop said CRLF was used instead of LF, my IDE said everything was in LF when I was doing these changes. Hopefully that is not an issue and can be corrected if necessary.

[magicono43]

Also just a FYI, I have not tested these changes, I mostly wanted to get this PR out there for opinions and critique, etc. If it seems practical and low-impact then continue from there hopefully.

[petchema]

If I understand the logic correctly, what this does is allow a mod to disable some "roll the dice" damage computations, most likely because they're replaced by another mechanism? If so, I suggest giving more accurate names than "Alter..." (say "Bypass..." or "Skip..."?), given the effect is to actually disable something (replacing them with something else is what the mod does, not those functions). Adding some comments to explain the purpose of the new functions is also always a good thing.

As a second thought, I wonder if it wouldn't be simpler (hence easier to understand), and yet more flexible, to allow to override the whole calling function (ApplyDamageToPlayer, etc.), so your mod could disable them by overriding them with a NoOp, but also other mods could also replace them with another computation. As usual they're also drawbacks (what happens if we fix bugs in core, what happens if more than one mod wants to modify those functions,...) and I'm not very familiar with modding in general, so this is at best just an idea. You may even have dismissed it for good reasons that I failed to consider.

[magicono43]

If I understand the logic correctly, what this does is allow a mod to disable some "roll the dice" damage computations, most likely because they're replaced by another mechanism? If so, I suggest giving more accurate names than "Alter..." (say "Bypass..." or "Skip..."?), given the effect is to actually disable something (replacing them with something else is what the mod does, not those functions). Adding some comments to explain the purpose of the new functions is also always a good thing.

As a second thought, I wonder if it wouldn't be simpler (hence easier to understand), and yet more flexible, to allow to override the whole calling function (ApplyDamageToPlayer, etc.), so your mod could disable them by overriding them with a NoOp, but also other mods could also replace them with another computation. As usual they're also drawbacks (what happens if we fix bugs in core, what happens if more than one mod wants to modify those functions,...) and I'm not very familiar with modding in general, so this is at best just an idea. You may even have dismissed it for good reasons that I failed to consider.

I really just put it where I did because it seemed least likely to be effected by future updates to that particular area of code, while also being able to skip over the rest of the code in that method, if the modder wanted to. I'm open for suggestions obviously, besides changing the name as you mentioned, I can't really think of a better way in this case to achieve what I'd like here. In my case I'd like to be able to change as much of the original combat function as I'm allowed, especially things like the knockback and when sound are played, etc.