OpenComputers
OpenComputers copied to clipboard
Port OpenComputers to Minecraft 1.16.5
This is a port of the 1.12 branch (as it appeared in May) for Minecraft 1.16.5. I've done some testing on the OC features to make sure they work (the mod is very much playable), but it's a big mod so I haven't tested every case yet (similar with mod compat: it works but it might behave differently in some cases). There are some things I've had to change, for example, GUI input works differently in Minecraft and so the semantics of the keyboard are different as well. Because of this, OPPM packages might be incompatible (but I've not removed/looked into that issue yet) and there have been some changes to the API. By the way, I'm making this as a pull request for the 1.12 branch (because that's what it's based on) but the intent is to create a new branch.
Impressive work! However, there's a bit of a problem... (a) payonel and I are both effectively stuck in 1.12.2 land, not willing to move beyond it; (b) Sangar is probably more willing to push OpenComputers 2 on 1.16+ as the de-facto sequel. However, I'll ask around.
Very very cool, thanks so much for taking the time to do this. As asie said, it's a lot, I'll try to help review this, but it'll take some time.
Assuming this PR is good (which it looks like from what I've seen so far!): as asie said, the other maintainers will probably stick to pre-1.16. Would you be able to/interested in maintaining the 1.16.5 version going forward?
Very very cool, thanks so much for taking the time to do this. As asie said, it's a lot, I'll try to help review this, but it'll take some time.
Assuming this PR is good (which it looks like from what I've seen so far!): as asie said, the other maintainers will probably stick to pre-1.16. Would you be able to/interested in maintaining the 1.16.5 version going forward?
Sure, I'm willing to continue working on it.
Personally speaking, I'm willing to merge this as soon as:
- OC1 API compatibility is restored (even if a bit rough around the edges),
- @payonel and @fnuecke approve the changes.
Not all bugs need to be fixed, just getting a beta release for 1.16.5 out would be magnificent.
Regarding the new in-game manual: I would absolutely move over to it, especially as the pre-1.16 manual has known bugs. This does not have to be part of the initial PR, however.
Regarding 1.12: The changes made in master-MC1.12 are significant in my opinion, and I would still wish to continue maintaining 1.7.10 if something is reported back from GT:New Horizons - they're still contributing features from 1.7.10. Therefore, I do expect Kosmos to be able to merge in code from the MC1.12 branch from time to time - however, this does not need to be part of the initial release or PR; it should be done soon after, though.
I do not foresee myself or GT:NH doing additional major changes that don't involve mod compatibility, however; I just don't have the time, and GT:NH primarily focuses on mod compat - and the number of mods which have a remotely similar API in 1.7.10 and 1.16.5 is vanishingly small :-)
Re branch: got it. Created a branch this can target based on the last commit before this PR for now then. Can be updated as a follow up.
Edit: fixed branch name.
Oh, one more thing: I'm fine with the rest of the changes in 1.12 since back then coming later, but could you please pick over the Github workflows/actions for PR validation and publishing? (i.e. the contents of /.github/workflows/ on current 1.12)
Thanks.
[23:25:06] [Render thread/WARN] [OpenComputers/]: Invalid glyph char code detected in font.hex. Expected 0<= charCode <10000, got: 1D300
[23:25:06] [Render thread/WARN] [OpenComputers/]: 87 total invalid glyph char codes detected in font.hex
This is fine. OpenComputers got font upgrades without getting patches to support Unicode codepoints outside the basic multilingual plane (BMP) - OC 1.8.0 has the necessary patches (they required reworking a bunch of Unicode logic and TextBuffer code, so they were kept for a new "big" release instead of an OC 1.7.x patch release), but Kosmos didn't pull them in yet.
Either way, personally, I'd say the keyboard thing remains a serious blocker - we don't want people to start writing incompatible OC1(1.16.5) code that doesn't work with OC1(1.7.10) or OC1(1.12.2), where possible to avoid. However, that remains the only merge blocker in my opinion.
- Creative tablets' screens don't render, but the rest are fine. I'm guessing the client giving it the wrong buffer ID somehow but I'm tracking that.
- As far as I know tablets turn off when they pass through being serialized (e.g. save game, get put into a chest for too long). I'd prefer them not do that but it appears consistent with 1.12 behavior.
- Noted, I planned to use the official FakePlayer for them but should at least get placing & mining blocks working.
- Noted, apparently toggling faces doesn't send the update required for cables to update their block state.
- I tested on a 'real' server and forgot to add it, should be a simple fix. I should eventually look into the data generator task also.
- Robots are indeed transparent even though the black square in the center makes it seem like they shouldn't be.
I agree that the keyboard input issue is one we need to resolve first (especially because I haven't merged the code which translates to LWJGL 2 codes yet).
There's some fixes for tablets in the later commits, FYI.
FTR, the robot render thing: if it's also like this in 1.12, I'm guessing the render mode of the "running effect" changed from "emissive opaque" to "emissive translucent" at some point before. Possibly during a previous port. Originally it wasn't see-through. E.g. see the trailer... has it really been 8 years? Damn.
I've made a tracking pull request for the keyboards feature. With the simpler changes already in, KosmosPrime/OpenComputers#1 should be the only blocking issue (for the initial pull anyway).
The keyboard changes are now in, so everything's OK from my side.
Regarding the changes since I started working on this - should I prefer to cherry-pick (and edit) commits (as I have with the workflow stuff) or merge them?
Merge. In general, changes in OpenComputers are applied from lowest applicable version to the most recent supported, so you want to be up to speed for future merges as well.
Anyhow, this is almost good for me! The only issue is the small smidgen of Lua changes - we'd want to merge them from 1.7.10 upwards if they are applicable to older versions, or get rid of them altogether if they're no longer necessary.
I've been trying out builds of this PR. I noticed that there is no LuaBIOS EEPROM (item or recipe), and the same is true of the different floppies such as the OpenOS one. Is this supposed to be the case?
I've been trying out builds of this PR. I noticed that there is no LuaBIOS EEPROM (item or recipe), and the same is true of the different floppies such as the OpenOS one. Is this supposed to be the case?
I've just tested with the most recent automatic build, both Lua BIOS and OpenOS are present and working (though it looks like the recipe for the Lua BIOS EEPROM doesn't). Could you elaborate on which build you used and how you've found these things to be missing (and whether other parts of the mod do exist)?
Could you elaborate on which build you used and how you've found these things to be missing (and whether other parts of the mod do exist)?
I am currently using the build from here: https://github.com/MightyPirates/OpenComputers/actions/runs/3406556101
I have had the same issue with running the build locally on abfd464.
I'm running it in a fresh instance of 1.16.5 in PolyMC with Forge version 36.2.34, with Scorge 3.1.3 also installed.
Neither the LuaBIOS EEPROM variant or floppy variants show up. I do have the base eeprom and base floppy items showing:
Is there something that I might be missing?
Edit:
Just to add, I also noticed these errors in the log:
[18:42:03] [Render thread/WARN] [OpenComputers/]: Attempt to get ItemStack for block Block{opencomputers:robotafterimage} without item form
[18:42:05] [Worker-Main-13/WARN] [minecraft/ModelBakery]: Unable to load model: 'opencomputers:floppy#inventory' referenced from: opencomputers:floppy#inventory: java.io.FileNotFoundException: opencomputers:models/item/floppy.json
Looks like they don't appear in the search tab, but do in the OC tab. I believe Minecraft builds the list to search before the server has a chance to send floppies to the client, but the EEPROM should be present from the start. Regarding the second error, I'm not sure why the game insists on loading that but it's never used, floppies exclusively use colored models.
Understood, sorry for the confusion, I should have looked in the OpenComputers tab! I can confirm that everything is there as it should be. When I couldn't craft the BIOS, and it wasn't showing up in search like it used to, it threw me off a bit.
Don't worry about either of those two issues, for the time being. The only objection I have is ensuring any changes to the Lua code (OpenOS etc.) are either undone or propagated from 1.7.10 onwards.
There's currently 4 lua changes relative to the branch point:
- A change in data's
nc.luawhich stops it from sending unprintable keys. It used to be more critical when pressing printable keys generated an extra event without a character attached, but the original code still causes NULs to be sent. - Two cases related to hard-coded key values to use constants instead. This is no longer relevant since we're using old key codes again.
- Part of the Lua upgrade commmit which I've included (the version number change) because I included the new library build pipeline but not the Lua 5.4 integration (yet). The 1.16 branch matches the 1.12 branch as it is now though, so safe to leave I think.
In that case, I'd do the following:
- split out the
nc.luachange into a separate PR, targettingmaster-MC1.7.10, it will make its way tomaster-MC1.16in due time; - whether or not you include the hard-coded key values to constants change is up to you, it's a code clarity thing I guess;
- the Lua copyright date change can stay, it will fix itself in due time as above.
Thank you!
I'm waiting on payonel to analyze the text_input event addition in light of OpenOS's architecture; he'll do his best to get it done this weekend. Once that is done (and the text_input event is added to 1.7.10/1.12.2 branches), it's a merge.
small issue in build https://github.com/MightyPirates/OpenComputers/actions/runs/3406556101 robot names dont show on the one probe
@KosmosPrime i'm really excited about this work. thank you a couple of things i found
- I noticed while typing in the shell (testing in openos) that some keys are not printed. I found that spamming a key sometimes sends char:0 instead of the correct char. Example, 'a' should send '97 30', but sometimes sends '0 30' (note the code is always correct. I noticed this char:0 only happens for keys that also signal text_input. char:0 is sent about 20% of the time
- it seems no text commands are registered. is this a known issue?
- I can't replicate this behavior (on windows), so I assume you're on unix, and I don't have a machine around to test it. In any case, my best guess (if the
text_inputevent appears still) is that the GLFW character event gets delivered a frame too late and the code fails to correlate them (this is why I've advocated for thetext_inputevent in the first place).
- I noticed while testing this that I accidentally broke key repeat event handling (fixed on catchup branch).
- I've not looked into commands yet, they didn't seem important at the time while requiring a significant rework for 1.16.5's autocomplete system. I'll definitely make a follow up including those.
yeah i was going to mention the repeat bug too. glad you noticed
yes i'm on linux, i'll retest on windows
I've submitted a small fix to allow using this on newer versions of Java (i.e. 11). https://github.com/KosmosPrime/OpenComputers/pull/2
I noticed while testing this that I accidentally broke key repeat event handling (fixed on catchup branch).
Keys still seem not to repeat properly on catchup-MC1.16. When using the edit program I was unable to hold down an arrow key to keep scrolling (it only registers as one press).
Is that the best branch to be testing on/working with? And on that note, is there anything I could do to help expedite this?