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

[Magicylsm] Audit Magiclysm Organization #78931

Merged
merged 8 commits into from
Jan 5, 2025

Conversation

QuillInkwell
Copy link
Contributor

@QuillInkwell QuillInkwell commented Jan 3, 2025

Summary

Mods "Audit Magiclysm Organization Part 1"

Purpose of change

Okay hear me out:
From a organizational stand point Magiclysm is kind of mess right now. I get that's probably just the nature of open source projects. But I felt it would be prudent to take a moment to refresh and re-organize things a bit and take out some of the worst offenders. Like the traits folder that has no clear defined delineation between mutations. Or EOCs being stored in the effects folder. Or the multiple instances of just blatantly redundant files like "Qualities" and "Tool_Qualities".

Describe the solution

Hope this isn't too much for one PR but here we go! (Sorry Maleclypse!)
Remove Redundant Files:

  • qualities.json has been merged into tool_qualities.json

Re-organize Terrain and Furniture

  • furniture.json has been merged into furniture.json in the furniture_and_terrain folder
  • terrain.json has been moved into the furniture_and_terrain folder

Sort out Effects and Effects on Conditions

  • Spellcasting related eocs have been moved to the spellcasting sub folder of the effect_on_conditions folder and had redundant prefixes removed.
  • All eocs currently living in the effects folder have been moved to the effect_on_condition folder and had their prefixes removed.
  • eoc_effect_remover.json moved.
  • eoc_forge moved and renamed to forge_of_wonders.
  • eoc_game_start moved.
  • eoc_transformation moved.

Re-organize Migration and Obsoletion

  • obsolete folder was renamed to migration_and_obsletion
  • obsolete.json has been removed, the spell contained within was moved into spell.json in the migration_and_obsoletion folder
  • migration_and_obsoletion.json has been renamed to migration.json. All obsoletions contained within have been moved into their correct .json (spells.json/item.json) files in the obsoletion_and_migration folder.
  • obsolete.json (in the items folder) has been spilt up into the existing migration and obsoletion files and thus removed.

Re-organize the Mutations folder and nuke the traits folder

  • Manatouched.json moved from traits to mutation_paths sub folder in the mutations folder
  • mutations.json (containing almost exclusively black dragon mutations) renamed to black_dragon.json and moved into the mutation_paths folder
  • black_dragon_category.json merged into black_dragon.json
  • mutation.json (containg mostly magic related mutations) moved from traits to mutations folder and renamed to mutations.json
  • Iron Allergy mutation moved from black_dragon.json to mutations.json
  • mutation_ecos.json moved to Effect_On_condition folder
  • mutation_enchantments.json moved to Enchantments folder
  • mutation_spells.json moved to Spells folder
  • attunements.json moved from traits to magic_classes sub folder in mutations folder
  • classes.json moved from traits to magic_classes sub folder in mutations folder
  • school_deficiencies .json moved from traits to mutations folder
  • infrastructure.json moved form traits to mutations folder and renamed to spell_triggering_mutations.json
  • magical.json renamed to shapeshifter_mutations.json
  • temporary_demon_traits.json merged into shapeshifter_mutations.json
  • temporary.json renamed to spell_invoked_mutations.json
  • fantasy_species.json (containing the mutations) was moved to the mutation_paths folder
  • fantasy_species.json moved from traits folder to Magicylsm folder. Renamed to hobbies_fantasy_species
  • deleted traits folder

Describe alternatives you've considered

I considering sorting out more of the folders but for now this feels like already too much. I got a little carried away. Again so sorry.

I considered splitting up mutations.json into a sort of magic_mutations.json that just contained mana related mutations and merging the school_deficiencies mutations into that. but opted against it. Primarily because it would orphan some mutations like Iron_allergy. I also just generally feel the school deficiencies are sufficiently different to stand in their own file.

Magic classes and Mutation paths don't strictly need their own subfolders but the mutation folder is pretty crowded. While I don't think we will get any more files related to classes, it could be possible we get more mutation paths in the future.

Fantasy Species aren't exactly mutation paths in the traditional sense but they are close enough in nature I felt it was appropriate to move them into the mutation paths folder. But I could have left them in the larger mutations folder.

I could have left the demo traits in their own file but I felt there was enough cross-over with the shapeshifting mutations to put it in there. Though it could also fit into the spell invoked mutations since it's not a full animal shapeshift like the others in that file.

I could have left the traits folder in. But the core game doesn't really draw a line between traits and mutations (to my knowledge). And the line was already getting blurred with things that were quite obivously mutations living in the traits folder. Collapsing everything into mutations removes the need to maintain this weird blurry line and just call all mutations as they are.

I strongly considered merging techniques.json and fantasy_species_techniques.json but felt each had their own differences sufficient to maintain the separation. Primarily for ease of adding future fantasy species.

The shapeshifting could technically fall under the umbrella of spell invoked mutations but I kept them separate. The shapeshifting mutations have a decent amount of heft to them and there's enough for me to want to keep them separate. But we could defiantly merge those files if we wanted.

I could always sort things further with greater levels of folders but I was trying not to go folder crazy creating way more organization than is currently needed.

Testing

All I did was move stuff around and rename stuff. So my testing primarily consisted of launching a world with Magicylsm to check for syntax errors like any orphaned commas or brackets. (It was fine)

Additional context

@github-actions github-actions bot requested a review from KorGgenT January 3, 2025 19:37
@github-actions github-actions bot added [JSON] Changes (can be) made in JSON Mods Issues related to mods or modding Mutations / Traits / Professions/ Hobbies Mutations / Traits / Professions/ Hobbies Monsters Monsters both friendly and unfriendly. Fields / Furniture / Terrain / Traps Objects that are part of the map or its features. Mods: Magiclysm Anything to do with the Magiclysm mod Mechanics: Enchantments / Spells Enchantments and spells EOC: Effects On Condition Anything concerning Effects On Condition astyled astyled PR, label is assigned by github actions json-styled JSON lint passed, label assigned by github actions labels Jan 3, 2025
@github-actions github-actions bot removed the json-styled JSON lint passed, label assigned by github actions label Jan 3, 2025
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@github-actions github-actions bot added the json-styled JSON lint passed, label assigned by github actions label Jan 3, 2025
@Maleclypse
Copy link
Member

Normally the answer is don’t organize. But magiclysm may be a special case as there are quite a few files I recall that have the same types of things stuffed in them to the point new contributors ask which file to use. I’ll review this

@QuillInkwell QuillInkwell changed the title [Magicylsm] Audit Magiclysm Organization part 1 [Magicylsm] Audit Magiclysm Organization Jan 4, 2025
@QuillInkwell
Copy link
Contributor Author

QuillInkwell commented Jan 4, 2025

I can totally see why it would be undesirable for people to be trying to organize things all the time. And I can rein back my ambitions and stop at this PR.

I think I got all the big things and glaring offenders. I did mildly want to touchup how the mapgen folder is structured but it's not a big deal. More nitpicking since I am probably going to be living in that folder for awhile while I work on maps for Magicylsm.

Next up is more variants of the magic cabin we all know and love. 😀

Oh and before I forget: Thank you for agreeing to review the PR.

@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Jan 4, 2025
@Maleclypse Maleclypse merged commit f62355c into CleverRaven:master Jan 5, 2025
23 of 26 checks passed
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 EOC: Effects On Condition Anything concerning Effects On Condition Fields / Furniture / Terrain / Traps Objects that are part of the map or its features. [JSON] Changes (can be) made in JSON json-styled JSON lint passed, label assigned by github actions Mechanics: Enchantments / Spells Enchantments and spells Mods: Magiclysm Anything to do with the Magiclysm mod Mods Issues related to mods or modding Monsters Monsters both friendly and unfriendly. Mutations / Traits / Professions/ Hobbies Mutations / Traits / Professions/ Hobbies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants