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

[Magiclysm] Partially bugfix spellcasting proficiencies #72149

Merged

Conversation

rty275
Copy link
Contributor

@rty275 rty275 commented Mar 3, 2024

Summary

Mods "Fixes spellcasting proficiencies adding when they should be subtracting"

Purpose of change

I noticed doing another PR that Apprentice and Master level proficiencies were doing the opposite of what they should have done in the specific case of being used as cost/time reductions.

Describe the solution

Change some + to -.

Describe alternatives you've considered

Letting the proficiencies keep working against you?

Testing

Checked both before and after, spells were properly adjusted by the math after the fix.

Additional context

During testing, I also noticed that Channeling proficiencies had a significantly lesser impact than other proficiencies had? Or maybe Evocation is just that powerful since it's working with smaller numbers. Dunno, but it's maybe worth looking into by someone who understands the math better than I do.

Edit: This does NOT deal with #72428, that's outside my level of coding ability to handle.

@github-actions github-actions bot added [JSON] Changes (can be) made in JSON Mods Issues related to mods or modding Mods: Magiclysm Anything to do with the Magiclysm mod labels Mar 3, 2024
@github-actions github-actions bot requested a review from KorGgenT March 3, 2024 19:10
@github-actions github-actions bot added <Bugfix> This is a fix for a bug (or closes open issue) json-styled JSON lint passed, label assigned by github actions astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions labels Mar 3, 2024
@Cadvin
Copy link

Cadvin commented Mar 4, 2024

Regarding the additional context: it seems to be because the bonuses from the proficiencies are operating via addition. With Novice Evocation, an evocation spell gets a +10 to its base damage, its maximum damage, and probably most impactfully, its damage increment per level (meaning most evocation spells hit damage cap quickly).
Channeling spells, meanwhile, get a -10 to the moves it takes to cast them, or enhancement spells a +10 to their duration in moves, which doesn't end up being much when these measure in the thousands or tens of thousands.

It should probably add a multiplier of the values instead (plus or minus 0.3, or something similar). I'd be happy to do a PR for this as my first contribution but I'll need to figure out how to work git properly first.

@rty275
Copy link
Contributor Author

rty275 commented Mar 4, 2024

That makes a lot of sense. It's mostly notable on the spells with either very low baseline numbers or very high ones. Magic Missile doing twice its base damage is cool, but perhaps more than being an expert at the spell would mean? Or maybe that is intended, I'm not sure, I wasn't the one who designed the proficiency system. And for spells with high numbers, the proficiencies are not very helpful because they're only removing a dozen or two seconds from an hour long spell. I have absolutely no idea how one would fix this, that kind of math and specifically coding is above me, but it is something I've noticed recently. I really like the proficiency system overall though, it feels more like learning how to wizard than reading spellbooks does.

@MNG-cataclysm
Copy link
Contributor

That makes a lot of sense. It's mostly notable on the spells with either very low baseline numbers or very high ones. Magic Missile doing twice its base damage is cool, but perhaps more than being an expert at the spell would mean? Or maybe that is intended, I'm not sure, I wasn't the one who designed the proficiency system. And for spells with high numbers, the proficiencies are not very helpful because they're only removing a dozen or two seconds from an hour long spell. I have absolutely no idea how one would fix this, that kind of math and specifically coding is above me, but it is something I've noticed recently. I really like the proficiency system overall though, it feels more like learning how to wizard than reading spellbooks does.

I think the earlier-suggested idea of a multiplier would be the best fix, as it allows for scale on spells. I also did not realize just how powerful the per-level damage addition would be, so that should probably have a scale of 0.1 or something similar.

@rty275
Copy link
Contributor Author

rty275 commented Mar 4, 2024

Multiplier makes sense, and .1 per tier of proficiency makes sense. I suppose in theory the most flexible solution would be to be able to set how effective the proficiencies are in the actual spell itself rather than as a universal constant, but that sounds like vastly overcomplicating things for very very little actual benefit.

@rty275 rty275 changed the title [Magiclysm] Bugfix spellcasting proficiencies [Magiclysm] Partially bugfix spellcasting proficiencies Mar 16, 2024
@Maleclypse Maleclypse merged commit cba18c3 into CleverRaven:master Mar 29, 2024
23 of 24 checks passed
@rty275 rty275 deleted the bugfix-spellcasting-proficiencies branch March 29, 2024 04:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions <Bugfix> This is a fix for a bug (or closes open issue) [JSON] Changes (can be) made in JSON json-styled JSON lint passed, label assigned by github actions Mods: Magiclysm Anything to do with the Magiclysm mod Mods Issues related to mods or modding
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants