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

Monitor feature #650

Draft
wants to merge 5 commits into
base: dev/0.8
Choose a base branch
from
Draft

Conversation

zyxkad
Copy link
Collaborator

@zyxkad zyxkad commented Jul 26, 2024

PLEASE READ THE GUIDELINES BEFORE MAKING A CONTRIBUTION

  • Please check if the PR fulfills these requirements
  • The commit message are well described
  • Docs have been added / updated (for features or maybe bugs which were noted). If not, please update the needed documentation here. Feel free to remove this check if you don't need it
  • All changes have fully been tested

@SirEndii SirEndii added enhancement New feature or request 1.19x labels Jul 27, 2024
@SirEndii SirEndii added this to the 0.8r milestone Jul 27, 2024
@SquidDev
Copy link
Contributor

Apologies for barging in here. I don't really like to do this, but I do think this is important. When copying code from another project, you need to follow the license that code is released under. Personally, I'm not normally fussed for small stuff, but this PR contains ~2k LoC copied from CC/CC:T.

In this case, it looks like the code has been copied from the 1.19.2 branch of CC:T, and so is licensed under the CCPL. The CCPL is a viral license, and requires that all projects using its code are also licensed under the CCPL.

I'd recommend instead taking as much code as possible from the 1.20.1 branch1. More of the code there is licensed under the MPL 2.0 which, unlike the CCPL, is non-viral. However, some pretty significant components are still under the CCPL (text rendering, monitor BE). For those, you have several options:

  • Ignore the license issues (please don't do this).
  • Relicense the whole project under the CCPL (definitely don't do this).
  • Ask Dan about relicensing the monitor code. I've heard he's more accessible these days, so might be worth a shot!
  • Perform a clean-room implementation of this code.

Footnotes

  1. I'd recommend doing that anyway, as there's lots of small bug fixes in it. For instance, TerminalState doesn't really need compression.

@zyxkad
Copy link
Collaborator Author

zyxkad commented Jul 28, 2024

@SquidDev Thanks for the LICENSE suggesting. (I'm not experience at it.) Just to confirm, if I paste the codes from 1.20.1 branch, which are under MIT license, is there no risk that will ask to change the LICENSE for all codes under AP? Also do I need to mention and keep the MIT license for those files that copied?

@SquidDev
Copy link
Contributor

Yeah, software licensing is very confusing! I find Choose a License and TLDR;Legal both pretty useful resources for summarising what a license enables/prevents you from doing.

For the MPL 2.0 (I assume you meant this rather than MIT, CC:T does not use MIT), the main requirement is that code licensed under the MPL retains that license. So you just need to keep the copyright header intact.

@zyxkad
Copy link
Collaborator Author

zyxkad commented Jul 29, 2024

Unfortunately a few files are still under CCPL (such as ServerMonitor), so I probably need rewrite some logics.

@zyxkad
Copy link
Collaborator Author

zyxkad commented Aug 5, 2024

Unfortunately a few files are still under CCPL (such as ServerMonitor), so I probably need rewrite some logics.

Okay, I probably have to use mixins, hopefully that won't break things.

@zyxkad
Copy link
Collaborator Author

zyxkad commented Sep 23, 2024

This PR is abandoned for two months, just to clarify, there are two problems need to solve.

  1. The monitor will block other transparent texture behind it. We maybe should fix the shaders, but I do not know how to fix. :/
  2. Most code is copy-pasted from CC:T under a strict license. The classes used final keyword to make extend impossible. So either waiting CC:T updated the relative files to its a less strict license or try work with mixin.

@SirEndii
Copy link
Member

This PR is abandoned for two months, just to clarify, there are two problems need to solve.

  1. The monitor will block other transparent texture behind it. We maybe should fix the shaders, but I do not know how to fix. :/

Transparent rendering is just pain in minecraft, it's a bit better in newer versions but still not nice to play with
I would just say that this is a bug which is currently at least for 1.19.2 unfixable

  1. Most code is copy-pasted from CC:T under a strict license. The classes used final keyword to make extend impossible. So either waiting CC:T updated the relative files to its a less strict license or try work with mixin.

CC:T does not support 1.19.2 anymore and I hope I can drop support for 1.19.2 too when I finish everything for 0.8
So that will not happen. So I guess you try it using mixins, or we implement that feature in newer minecraft versions after I ported 0.8 to these versions

@SirEndii SirEndii mentioned this pull request Oct 24, 2024
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.19x enhancement New feature or request
Projects
Status: In Progress
3 participants