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

feat(content): In-Repo Magical Nights, replace Magiclysm #5881

Merged
merged 11 commits into from
Jan 6, 2025

Conversation

RobbieNeko
Copy link
Collaborator

Checklist

Required

Purpose of change

Magiclysm has been abandoned after I left it to work on Magical Nights under my own vision. Magical Nights is regarded as better-balanced, more generally up-to-date, and has more content than Magiclysm. The fact that Magical Nights was out of the BN repo has in the past been noted as a friction point by new users, and thus it is natural to in-repo the mod to replace Magiclysm.

Describe the solution

  • In-repo's Magical Nights
  • Obsoletes Magiclysm
  • Adds Magical Nights' license (CC BY-SA 4.0) to the license document
  • Adds Magical Nights to the valid semantic scopes
  • Blacklists Magiclysm from mods testing

Describe alternatives you've considered

  • Leave Magiclysm unobsoleted

It's just straight up not maintained anyway, and better for it to gracefully retire than sit around festering.

  • Implode
  • Give Chaos a bit more time to try and in-repo Arcana first
  • Leave Magiclysm out of the testing blacklist

Why? It's obsoleted, and any actual testing needs relating to magic and the like should be handled just fine by testing Magical Nights instead.

Testing

The JSON worked before, if it suddenly breaks because of copy-pasting the folder I will eat a shoe.

Everything should be linted and I caught all the single-space-after-period potential errors I think, but we will see if the tree proves me foolish (or the build tests)

Additional context

Magical Nights looks to be winning the in-repoing race against Arcana ;P

RobbieNeko and others added 7 commits January 5, 2025 11:44
Co-authorship provided to be extra safe on attribution

Co-Authored-By: Chorus System <[email protected]>
Co-Authored-By: Ithilrandir <[email protected]>
Co-Authored-By: arijust <[email protected]>
hopefully this isn't too controversial
We actually do keep obsolete mods in the semantics, who knew
Any testing needs should be covered by Magical Nights anyway
@github-actions github-actions bot added docs PRs releated to docs page JSON related to game datas in JSON format. mods PR changes related to mods. labels Jan 5, 2025
Copy link
Contributor

autofix-ci bot commented Jan 5, 2025

Autofix has formatted code style violation in this PR.

I edit commits locally (e.g: git, github desktop) and want to keep autofix
  1. Run git pull. this will merge the automated commit into your local copy of the PR branch.
  2. Continue working.
I do not want the automated commit
  1. Format your code locally, then commit it.
  2. Run git push --force to force push your branch. This will overwrite the automated commit on remote with your local one.
  3. Continue working.

If you don't do this, your following commits will be based on the old commit, and cause MERGE CONFLICT.

@RobbieNeko
Copy link
Collaborator Author

Darn, the tree found linting errors. Oh well, tis why it is there

RoyalFox2140
RoyalFox2140 previously approved these changes Jan 5, 2025
Copy link
Collaborator

@RoyalFox2140 RoyalFox2140 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IF IT LOADS, IT MERGES

I mean people have been playing with MN out of repo for a few months and people seem to prefer it universally. Korg isn't here to work on his rotting-on-the-vine expansion and in the past two years we haven't seen any real work on it.

Oops
@Dread-Pirate-Hogarth

This comment was marked as off-topic.

@Dread-Pirate-Hogarth

This comment was marked as off-topic.

@RobbieNeko
Copy link
Collaborator Author

RobbieNeko commented Jan 5, 2025

A. Are you attempting to suggest that Magical Nights should have the same mod ID as Magiclysm? That would be absurd.
B. No, it absolutely will not break any save games. Magiclysm is obsoleted, not deleted. It means that people won't be able to add Magiclysm to their new worlds, not that existing worlds will suddenly break. And I can certainly not imagine any situation where Magical Nights would break as a result of this.
C. There is a readme.md explaining the origins of the mod and even specifically listing out the authors marked in the original Magiclysm for BN. Once again, Magiclysm is obsoleted, not deleted.

@Dread-Pirate-Hogarth

This comment was marked as off-topic.

@RobbieNeko
Copy link
Collaborator Author

"The next json change will break all magiclysm saves" is deliberately hyperbolic. We rarely make changes that would cause a save to be unable to load.
But even if we were to make such a change, we also allow No Hope to break all the time because it's a mess that no-one wants to maintain. No Hope isn't even obsoleted!

As for attribution:
Magical Nights has existed for over a year at this point, and yet now is when you choose to bring up your concerns? It's not like Magical Nights was hiding for that year either, it's been a prominently recommended mod throughout the community, it's been in the mod-links channel in the BN Discord.

@RobbieNeko
Copy link
Collaborator Author

Additionally, they can see the commits made to Magiclysm in BN's past just fine. It's not like I'm hiding the parentage of Magical Nights.

@Dread-Pirate-Hogarth

This comment was marked as off-topic.

@Dread-Pirate-Hogarth
Copy link
Contributor

you will want to identify the magiclysm commit you branched from, likely shortly before the date of 7f0a3d28a6e4d94a59ee88047ad6ee8534114a3c

then create a diff between 7f0a3d28a6e4d94a59ee88047ad6ee8534114a3c and the magiclysm original commit, which you apply as a commit. after that you cherry pick (replay) all commits from your repo onto the branch. then cherry pick commits from this pr onto the branch. finally one last compatibility commit to change the mod id to magiclysm. and you are done.

@Dread-Pirate-Hogarth
Copy link
Contributor

@RobbieNeko would it help you if I made the branch and committed on my fork?

@RoyalFox2140
Copy link
Collaborator

This will not break any saves. Previous comments are off-topic due to being HIGHLY wrong. Mods flagged as Obsolete are the correct way to remove old broken and outdated content as it prevents new users from loading those mods while old save files will still run them.

Do not clog the PR with assumptions of problems, it's ok to ask but this is spreading of misinformation.

@Dread-Pirate-Hogarth
Copy link
Contributor

im sorry but that is not correct. obsolete mods are still loaded.

/** Obsolete mods are loaded for legacy saves but cannot be used when starting new worlds */
bool obsolete = false;

if obsolete mods werent loaded there would be no reason to have obsoletion. you would just delete the mod, because it would have the same effect.

@RoyalFox2140
Copy link
Collaborator

im sorry but that is not correct. obsolete mods are still loaded.

/** Obsolete mods are loaded for legacy saves but cannot be used when starting new worlds */
bool obsolete = false;

if obsolete mods werent loaded there would be no reason to have obsoletion. you would just delete the mod, because it would have the same effect.

Please read the previous message and kindly stop. I will not warn you again.

@scarf005
Copy link
Member

scarf005 commented Jan 5, 2025

@Dread-Pirate-Hogarth
marking magiclysm as obsolete will not break existing saves. obsoletion literally hides the mod from new world creation and nothing else. please provide proof that this PR breaks existing saves with magiclysm if you think otherwise.

@chaosvolt chaosvolt merged commit 0550f50 into cataclysmbnteam:main Jan 6, 2025
15 checks passed
@KorGgenT
Copy link
Contributor

KorGgenT commented Jan 6, 2025

hi! it's come to my attention that you have removed all authorship from magiclysm and copy pasted it into your own mod! this is both plagiarism and against the copywrite. this is the only warning i'll give that you must fix attribution or revert this change. i'll give it a week then check back.

@KorGgenT
Copy link
Contributor

KorGgenT commented Jan 6, 2025

the above comment is directed at the pr author in addition to the maintainers. i don't really follow BN too closely so i'm not sure who those are but i've been informed maybe @scarf005 ? please disseminate the information accordingly

@scarf005
Copy link
Member

scarf005 commented Jan 6, 2025

Hello @KorGgenT, I apologize for the incorrect attribution of the mod. As far as I understand, magiclysm is licensed under CC-BY-SA 3.0 like DDA; is that correct? Would there be any additional attribution requirements other than adding you to the authors field in modinfo.json?

@KorGgenT
Copy link
Contributor

KorGgenT commented Jan 6, 2025

yes, the original commit information via magiclysm. so rather than "new file" commits, it should be "moved file" commits if you really want to change the folder, and then change the mod name. because of the fgact that i am not the only author, there are many authors, that's the only reasonable way to do it.

@scarf005 scarf005 mentioned this pull request Jan 6, 2025
4 tasks
@RoyalFox2140
Copy link
Collaborator

yes, the original commit information via magiclysm. so rather than "new file" commits, it should be "moved file" commits if you really want to change the folder, and then change the mod name. because of the fgact that i am not the only author, there are many authors, that's the only reasonable way to do it.

Ok, this was brought to my attention. I thought the solution was adding authors but if you want to move the Magiclysm files this will sadly destroy the previous version for anyone still using it. This will mean that since Robbie has gone a different direction with Magical Nights the old gameplay will disappear.

@RoyalFox2140
Copy link
Collaborator

I'll take care of a revert now. These files are FUBAR due to Git commit history. If you say that moving the file is enough to maintain authorship we'll figure out how to add back the original Magiclysm version later.

@RoyalFox2140
Copy link
Collaborator

yes, the original commit information via magiclysm. so rather than "new file" commits, it should be "moved file" commits if you really want to change the folder, and then change the mod name. because of the fgact that i am not the only author, there are many authors, that's the only reasonable way to do it.

cc: @Coolthulhu

@KheirFerrum
Copy link
Collaborator

yes, the original commit information via magiclysm. so rather than "new file" commits, it should be "moved file" commits if you really want to change the folder, and then change the mod name. because of the fgact that i am not the only author, there are many authors, that's the only reasonable way to do it.

Github doesn't store per-file commit histories if I recall, it instead uses heuristics that read commits to put together a "history". So an alternative solution we could have is to make a "copy" of Magiclysm in the Magical Nights folder as a single PR that is then built upon in later PRs to make them distinct. This should satisfy the algorithm, though I still question if we could just use attribution in modinfo.json in terms of adding your group to the author list, as well as a clear description stating that this is based off Magiclysm (I believe there's already descriptive text about that.)

@Dread-Pirate-Hogarth
Copy link
Contributor

is there something wrong with diffing from magiclysm as I suggested? :\

@RoyalFox2140
Copy link
Collaborator

is there something wrong with diffing from magiclysm as I suggested? :\

Maybe we didn't take kindly to your trolling about saves being broken so we didn't listen to anything you said.

@KorGgenT
Copy link
Contributor

KorGgenT commented Jan 6, 2025

yes, the original commit information via magiclysm. so rather than "new file" commits, it should be "moved file" commits if you really want to change the folder, and then change the mod name. because of the fgact that i am not the only author, there are many authors, that's the only reasonable way to do it.

Ok, this was brought to my attention. I thought the solution was adding authors but if you want to move the Magiclysm files this will sadly destroy the previous version for anyone still using it. This will mean that since Robbie has gone a different direction with Magical Nights the old gameplay will disappear.

well i'm not sure how you guys do it, but if there is no maintainer for the magiclysm on BN, then really that should be the way it goes, right?

RoyalFox2140 added a commit that referenced this pull request Jan 6, 2025
Revert "feat(content): In-Repo Magical Nights, replace Magiclysm (#5881)"

This reverts commit 0550f50.
@RoyalFox2140
Copy link
Collaborator

well i'm not sure how you guys do it, but if there is no maintainer for the magiclysm on BN, then really that should be the way it goes, right?

Kenan expressed a desire for us to continue supplying Magiclysm and I agreed that as long as it worked and as long as we correctly flagged it as Obsoleted when it wasn't functional, that players could pick up the mod at any point they wished and maintain it. This would preserve the original balance, lore, and world of Magiclysm while Robbie was free to do anything she wished to change the original vision.

@Dread-Pirate-Hogarth

This comment was marked as spam.

@RobbieNeko
Copy link
Collaborator Author

RobbieNeko commented Jan 6, 2025

@KorGgenT Hello, KorG.
I must profusely apologize for my improper attribution. I was under the impression that the credit in the readme.md would be enough, especially given the intention to keep Magiclysm around as an obsoleted mod. However, I see now my error. I recognize that in particular, the lack of commit continuity is most definitely my fault, and I take full responsibility for it.

I am sorry that we have gotten off on the wrong foot, and I aim to remedy the issues you have raised.
I just have a question regarding the proposed solution you put forth:
Do you mean for Magiclysm to essentially get removed within our repo, and fully replaced with Magical Nights in the repo? i.e. not even obsoleting Magiclysm. (Instead of there being Magiclysm(Obsolete) and Magical Nights, there'd be just Magical Nights)
I'd have concerns here with breaking saves by doing so, and I was always working under the assumption of keeping Magiclysm around for posterity and so that people could see the base that I 'forked' off of in order to create Magical Nights.
Assuming you do mean that Magical Nights should completely take the place of Magiclysm, here is my current plan:

  • I create a new folder for Magical Nights (commit 1)
  • I transfer the files of Magiclysm into the new folder, and delete the old folder (commit 2, I'm splitting the creation of the folder and the moving of the files into separate commits to make as sure as possible that Github considers them to have been moved and thus preserves the commit history as best as I can)
  • I cherry-pick all the (relevant) Magical Nights commits from its repo and apply them into the new folder (commits 3 - N depending on how many commits there truly are)
  • I add the original authors listed in the Magiclysm modinfo.json (currently listed in the readme.md in Magical Nights) to the authors list alongside myself, at least until a majority of the content is truly Magical Nights content and not just inherited Magiclysm content. The readme.md, however, would not be changed unless it is to add more authors and would continue to list the authors listed for Magiclysm in our repo. (Commit N+1)

Do you believe this to be satisfactory?

@Ignaramico
Copy link

finally, now i dont have to go to its github to keep it updated

@RobbieNeko
Copy link
Collaborator Author

finally, now i dont have to go to its github to keep it updated

Bad news... you're gonna have to wait a bit longer, I'm afraid. But don't worry, it will be happening (as you can see above, there's concerns about attribution, so this PR actually got reverted relatively shortly afterwards)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs PRs releated to docs page JSON related to game datas in JSON format. mods PR changes related to mods.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants