khanshoaib3 / minecraft-access

A mod for minecraft java (fabric/neoforge) that adds accessibility feature
GNU General Public License v3.0
45 stars 10 forks source link

Legacy Version Support #130

Open Mythra opened 1 year ago

Mythra commented 1 year ago

Hi! Thank you so much for all your work here, it's been really awesome getting to play Minecraft again! One thing I'm curious about contributing code for is supporting older versions, specifically looking at versions like v1.16.1. I understand supporting these legacy versions may add in an undue burden to maintenance though so it may not be wanted. I really just wanted to open the discussion if you'd be willing to accept PRs that added in / fixed issues for 1.16.1 & potentially other legacy versions even if there was no guarantee of support. Or if I should be looking at a fork if I want to maintain some of these older versions. Thanks!

boholder commented 1 year ago

I'm happy to see new potential contributor, hi!

Me and @khanshoaib3 have made a decision that we only support two major version of the game. Currently we support 1.19 (1.19.3 in fact) and 1.20, and we'll drop (haven't discussed about whether bug-fix-only or totally stop supporting) 1.19 by next June when 1.21 comes out. To be honest, cherry-picking every commit to both two branches is already a bit of a pain for me (and for Shoaib) at the moment.

About developing mod on old versions

The Mojang dev team will refactor code as needed. We've only had passed through one major version update #60 , and that one broke two or three features of this mod. So I can imagine how many red errors you will see when you downgrade the development environment to 1.16.1. Not to mention that there are currently some features that do not exist in older versions.

IIRC, Shoaib used to developing accessibility mod on old versions of Minecraft. Maybe his legacy code can help you, IDK. Maybe stick on THIS mod's structure is better for future maintaining.

Fork or PR to new branch?

Technically, it wouldn't be too much of a burden for me to start a new branch in this project.

If we choose the PR option, we can exchange good ideas via review and cherry-pick or so. Maybe you'll be invited as collaborator in the future, then you can help reviewing our PRs, which is good for us.

If we choose the Fork option, everything bad (burden) and good will gone, the connection between the two projects will be weak, but more free.

I'm in favor of both options, maybe the PR option is better, IDK, sorry for my indecisiveness.

Waiting for @khanshoaib3 's comment.

Mythra commented 1 year ago

Hey I really appreciate the openness on this, and I absolutely understand the cherry-picking concerns and versions. I know there's not really one easy answer here. If you're okay with it, and it sounds like you might be, we may want to approach something like a "tiered support" strategy.

Where the latest two versions are "tier 1", and then several other versions may be "tier 2" where support & new features are released at their own slower rate & may have their own bugs (presumably the only barrier could be something like 'a maintainer is willing to review code reviews'). Taking this idea mostly from other oss projects (e.g. take rust's OS support:

Rust provides three tiers of target support: Rust provides no guarantees about tier 3 targets; they exist in the codebase, but may or may not build. Rust's continuous integration checks that tier 2 targets will always build, but they may or may not pass tests. Rust's continuous integration checks that tier 1 targets will always build and pass tests.

-- sounds like we don't have integration tests, but if we wanted to do branches and make separate guarantees, that does sound like a very viable option).

I am also happy to contribute some of the things I do into the main branch as well, and did plan on potentially adding some features to all versions too. So either way should hopefully see some PRs come through 😄

boholder commented 1 year ago

If you're okay with it, and it sounds like you might be, we may want to approach something like a "tiered support" strategy.

Yup, what I said is like the tiered support strategy or something similar.

This mod is sort of special:

I can't think of a similar project at the moment, maybe it's like other infrastructure projects (like programming languages), but less initiatives. Applying the tiered support strategy can prevent the maintenance cost snowballing.

Since upstream (game) updates will break mod features, we can't apply so called Trunk Based Development, this situation forces us to use a "bad approach" - working on different branches separately. This will prevent a lot of good git collaboration practices from being used in this program.

Since we can't easily merging future commits (new features, fixes) back to old branches, new commits need to be cherry-picked by the contributor him/herself synchronously, not merged by the maintainer asynchronously. Cherry-pick is cheaper than merge since every merge will expose all version-conflicts again, git doesn't know how to deal with them.

Or there are magic solutions that make the git to resolve these conflicts automatically. 🤔

-- sounds like we don't have integration tests, but if we wanted to do branches and make separate guarantees, that does sound like a very viable option).

I think branches (major game versions) can have four states in this project:

  1. Teir 1: New features + bug fixes
  2. Teir 2: Bug fixes (+ volunteering new-feature-port PRs)
  3. Teir 3: Stop supporting actively (+ volunteering new-feature-port PRs and bug-fix PRs)
  4. Teir 4: Stop accepting volunteering PRs (can't hold the cost of reviewing or so)

Except for the latest version should be placed in Teir 1, other versions are free to be placed in others, since there this no further game version update actively breaks them anymore.

I am also happy to contribute some of the things I do into the main branch as well, and did plan on potentially adding some features to all versions too. So either way should hopefully see some PRs come through 😄

Glad to hear that. And it reminds me of the Bus Factor. We can increase the Bus Factor by reviewing each other's PRs, I hope we can share the legacy versions mod code ownership with you by reviewing your code.

boholder commented 1 year ago

The current user base for this mod is still small and still slowly growing, and I'm not sure how the strategy of "supporting the second new version for an extra year" sounds to them... We haven't done user research (and don't know how to do it). Okay, I'll admit that my real intention is that I want to slack off, like changing "a year" to "six months" or "a quarter", just provide enough time for servers and single players to transition to the new version. 😂

But I don't want the support strategy to limit players choice on who to play with. There is one situation that a visual impaired player plays with sighted players in a server, but somehow the server admin refuses to update the server's version, or most of players don't want to update the version. In this case, the visual impaired player may be frustrated.

boholder commented 1 year ago

Hey @Mythra, you may want to wait until #116 issue is finished, this refactor will cause lots of changes in most of classes, which may cause trouble to your port. Sadly I lost most of work last week after an accidentally file deletion... (the .git files are corrupted and can't be recovered from the backup 😢) I'll try to finish it within this week.

Mythra commented 1 year ago

Hey, I appreciate the heads up! I do have some things locally, but they're not really ready for pushing up (I've been waiting to work on it til I got approval on the branch strategy). Also please feel free to take your time, not only because I don't want to rush you, but some other stuff has come up to. I won't be in a good emotional place to work on anything for a bit.

khanshoaib3 commented 1 year ago

@Mythra I'm with @boholder on this. I'm not experienced enough to know all the strategies y'all are talking about but yeah, use whichever strategy that you both think suits best. The previous mods did support 1.16 so ping me in case you need any help with porting. Additionally, you can look at the source code of the mods too although I admit the code base is a mess lol. https://github.com/accessible-minecraft

Mythra commented 1 year ago

@boholder when the PRs you mentioned with major changes merge, do you wanna create a branch I can submit PRs to, Say 1.16.1, and let me know? It'll probably still be a day or two before I'm ready to submit anything at all, but when i see that branch being created, and am feeling up to it I can start submitting some things.

boholder commented 1 year ago

@boholder when the PRs you mentioned with major changes merge, do you wanna create a branch I can submit PRs to, Say 1.16.1, and let me know?

Sure, which do you want, 1.16 or exactly 1.16.1? 1.16 is shorter so you can input two less letters every time you working on it IMO.

It'll probably still be a day or two before I'm ready to submit anything at all

Waiting for @khanshoaib3 to merge #138 #139 that contains #116 😉

Mythra commented 1 year ago

Sure, which do you want, 1.16 or exactly 1.16.1? 1.16 is shorter so you can input two less letters every time you working on it IMO.

I was thinking 1.16.1 specifically because i wasn't going to test 1.16 right away, just 1.16.1. I see we already have a branch though that is 1.21.X. So I'd say we either go with 1.16.1 explicitly, or if we want to support both, follow the existing pattern and use 1.16.X, I don't think the exact character count matters to me personally, and don't think there's a reason to break pattern here.

boholder commented 1 year ago

We... are preparing to change 1.20.x -> 1.20, default -> 1.19, sorry for leak of explanation 😂

Mythra commented 1 year ago

No worries at all!! very good to know! Then 1.16.1, or 1.16 is fine! 😄

khanshoaib3 commented 1 year ago

Merged the PRs 🙂.

boholder commented 1 year ago

👌 https://github.com/khanshoaib3/minecraft-access/tree/1.16.1