CC: Tweaked

CC: Tweaked

42M Downloads

[MC-next] Breaking API changes

SquidDev opened this issue ยท 11 comments

commented

This is mostly a todo list for breaking API changes scheduled for the next Minecraft version, be that 1.19.3 or 1.20:

Removals

  • Remove Forge-specific methods from ComputerCraftAPI.
  • Remove IComputerSystem.getFileSystem (maybe we should mark it as deprecated now). This has always been a little broken, and I don't think it gets any use.

Changes

  • Mount getters in ComputerCraftAPI (e.g. createUniqueNumberedSaveDir) should accept a MinecraftServer, not a Level. Possibly also true for the wireless network getter?

    1.19.3 also redoes registries a little, so maybe the detail registries should also accept a ServerLevel/MinecraftServer. Less sure there.

  • Rethink how IPacketNetwork and IWiredNode interact: the fact that IWiredNode extends IPacketNetwork has always been ugly, and I think there's probably better ways.

  • Redo IComputerAccess.mount entirely:

    This is a pre-requisite to #676: if we're going to move mounting logic to the Lua side (which I think we probably are) then we need the actual mounting operation to be asynchronous. As a result, the semantics of this API call need to change a bit:

    • The API call should always succeed: mounting will never "fail", it just may be ignored.

    • However, there's no guarantee that the mount will occur immediately (or at all). As such, we should return some AttachedMount interface, which contains a getMountPath(): String? method (as well as a close()/unmount() method).

    • The implementation should be responsible for handling conflicting paths (for instance, mounting disks at /disk2, etc... instead) - currently that's done by the caller.

      It's possible that some mods may want to guarantee something is mounted at a specific path (some mods used to mount additional programs when a peripheral was attached, though really that should be done with datapacks now), and not allow mounting it multiple times. Maybe we need an option for that?

  • Rethink IMount/IWritableMount: This is much less thought through right now, but some things I'd quite like to look into:

    • Can we make the API closer to Java's built-in Filesystem? Is there any merit in doing so?
    • Support moving files (maybe whole directories too?) within a mount (see #962).
    • Support opening files in r/w mode.
    • Make the current default methods non-default.
  • Rename ForgeComputerCraftAPI to ComputerCraftAPIForge.

commented

If we're doing breaking changes now, how about Lua 5.2 support? ๐Ÿ‘€

commented

I don't think Lua 5.2 has anything to do with the API

commented

But it does change behavior for user code, and some of those behavior changes might be labeled as breaking changes.

Although, now I'm wondering if it's a good idea to bundle an API rewrite with a bump in the language version. As much as I would like to see us using Lua 5.2 perhaps it's a good idea to not change too much at the same time.

But when are we next going to have an opportunity where CC is targeting few versions of MC for changing the language? Normally we have two at least and having the previous version of MC run 5.1 when latest has 5.2 seems like it'll extend any confusion that people have with differences.

commented

But it does change behavior for user code

I don't think anything in this list should affect CraftOS yet. It unlocks the door for some changes in the future, but for now just affects the API that external mods consume.

commented

Sorry, should have clarified. "[Lua 5.2] does change behavior for user code..." should probably have been how I started that sentence.

commented

Ahh, that makes more sense. Sorry, thank you for clarifying!

commented

Now that I think of it, some peripheral mods might change what they do/how they do things if we switch to Lua 5.2, but that's probably not going to be much of a bother for most peripheral mods.

commented

Something semi-relevant to the IMount changes I was going to write a PR for for 1.19.2 was to move up the isReadOnly method from MountWrapper to IMount to make it possible to do an actual readonly check on the underlying filesystem (for real Java filesystems) - really niche thing that we were finding useful to check on SC as we set computer dirs as read-only sometimes. Not sure how that would fit in with this or if you think it's worth including at all.

commented

@Lemmmy Oh, that's probably reasonable! Could probably do that now TBH: if it's a default method then it won't cause any breaking changes.

commented

It is tempting to switch to an in-memory filesystem, which syncs periodically to the disk (which we would need for State persistence and making complex programs possible #535 anyway). The main issue there is this makes editing computer files externally much harder. Not sure what the right solution is.

About external editing, I believe turning off the computer should immediately save the state to disk, instead of waiting for the next world save in order to do so. Also it could be an option in the config whether the in-memory filesystem is enabled or not, perhaps it could be implemented alongside persistence as an opt-in feature.

commented

Decided to push back the remaining changes to another version.

  • I'd forgotten why wired nodes and packet networks are the way they are (because messages are sent to/from the computer rather than the node, we have a separate object). That said, this feels like our internal design choices are leaking into the public API a little.

    I'm sure there's stuff we can do here to clean things up, just want some more time to do so. It's definitely not urgent.

  • IComputerAccess.mount: As discussed in #676, I'm not comfortable with how this would look on the Lua side yet, so no sense changing the Java API.

  • Support opening files in r/w mode: There's a lot of nastiness here in how to handle file-size tracking when the same file is open multiple times, which I'm not entirely sure how to fix in a cross-platform way.

    It is tempting to switch to an in-memory filesystem, which syncs periodically to the disk (which we would need for #535 anyway). The main issue there is this makes editing computer files externally much harder. Not sure what the right solution is.