This should never happen, but apparently it does!? We now log an error
(rather than crashing), and include the original BE (and associated
block), as the BE type isn't very useful.
See #1750. Technically this fixes it, but want to do some more poking
there first.
Here's a fun bug you can try at home:
- Create a new world
- Spawn in a pocket computer, turn it on, and place it in a chest.
- Reload the world - the pocket computer in the chest should now be
off.
- Spawn in a new pocket computer, and turn it on. The computer in chest
will also appear to be on!
This bug has been present since pocket computers were added (27th March,
2024).
When a pocket computer is added to a player's inventory, it is assigned
a unique *per-session* "instance id" , which is used to find the
associated computer. Note the "per-session" there - these ids will be
reused if you reload the world (or restart the server).
In the above bug, we see the following:
- The first pocket computer is assigned an instance id of 0.
- After reloading, the second pocket computer is assigned an instance
id of 0.
- If the first pocket computer was in our inventory, it'd be ticked and
assigned a new instance id. However, because it's in an inventory, it
keeps its old one.
- Both computers look up their client-side computer state and get the
same value, meaning the first pocket computer mirrors the second!
To fix this, we now ensure instance ids are entirely unique (not just
per-session). Rather than sequentially assigning an int, we now use a
random UUID (we probably could get away with a random long, but this
feels more idiomatic).
This has a couple of user-visible changes:
- /computercraft no longer lists instance ids outside of dumping an
individual computer.
- The @c[instance=...] selector uses UUIDs. We still use int instance
ids for the legacy selector, but that'll be removed in a later MC
version.
- Pocket computers now store a UUID rather than an int.
Related to this change (I made this change first, but then they got
kinda mixed up together), we now only create PocketComputerData when
receiving server data. This makes the code a little uglier in some
places (the data may now be null), but means we don't populate the
client-side pocket computer map with computers the server doesn't know
about.
- Remove "initial connections" flag, and just refresh connections +
peripherals on the first tick.
- Remove "peripheral attached" from NBT, and just read/write it from
the block state. This might cause issues with #1010, but that's
sufficiently old I hope it won't!
Our GatedPredicate hack was clever, but also fundamentally didn't work.
The predicate is called before extraction, so if extraction fails (for
instance, canTakeItemThroughFace returns false), then we still think an
item has been removed.
To fix that, we inline StorageUtil.move, specialising it for what we
need.
This feels a little overkill, but nice to standardise how this code
looks.
There's a bit of me which wonders if we should remove
IPeripheral.equals, and just use Object.equals, but I do also kinda like
the explicitness of the current interface? IDK.
This ensures the client decoder is in sync with the server. Well, mostly
- we don't handle the anti-jerk, but that should correct itself within a
few samples.
Fixes#1748
The original runtime error reporting PR[^1] added a "cc.exception"
module, which allowed coroutine managers (such as parallel) to throw
rich errors, detailing the original context where the error was thrown.
Unfortunately, the change to parallel broke some programs (>_>, don't do
string pattern matching on your errors!), and so had to be reverted,
along with the cc.exception module.
As a minimal replacement for this, we add support for user-thrown
exceptions within our internal code. If an error object "looks" like an
exception ("exception" __name, and a message and thread field), then we
use that as our error information instead.
This is currently undocumented (at least in user-facing documentation),
mostly because I couldn't figure out where to put it - the interface
should remain stable.
[^1]: https://github.com/cc-tweaked/CC-Tweaked/pull/1320
This adds support for computer selectors, in the style of entity
selectors. The long-term goal here is to replace our existing ad-hoc
selectors. However, to aid migration, we currently support both - the
previous one will most likely be removed in MC 1.21.
Computer selectors take the form @c[<key>=<value>,...]. Currently we
support filtering by id, instance id, label, family (as before) and
distance from the player (new!). The code also supports computers within
a bounding box, but there's no parsing support for that yet.
This commit also (finally) documents the /computercraft command. Well,
sort of - it's definitely not my best word, but I couldn't find better
words.
When rendering non-origin monitors, we would fetch the origin monitor,
read its client state, and then cache that on the current monitor to
avoid repeated lookups.
However, if the origin monitor is unloaded/removed on the client, and
then loaded agin, this cache will be not be invalidated, causing us to
render both the old and new monitor!
I think the correct thing to do here is cache the origin monitor. This
allows us to check when the origin monitor has been removed, and
invalidate the cache if needed.
However, I'm wary of any other edge cases here, so for now we do
something much simpler, and remove the cache entirely. This does mean
that monitors now need to perform extra block entity lookups, but the
performance cost doesn't appear to be too bad.
Fixes#1741
Historically, computers tracked whether any world-visible state
(on/off/blinking, label and redstone outputs) had changed with a single
"has changed" flag. While this is simple to use, this has the curious
side effect of that term.setCursorBlink() or os.setComputerLabel() would
cause a block update!
This isn't really a problem in practice - it just means slightly more
block updates. However, the redstone propagation sometimes causes the
computer to invalidate/recheck peripherals, which masks several other
(yet unfixed) bugs.
- colors.toBlit now performs bounds checks on the passed value,
preventing weird behaviour like color.toBlit(2 ^ 16) returning "10".
- The window API now uses colors.toBlit (or rather a copy of it) for
parsing colours, allowing doing silly things like
term.setTextColour(colours.blue + 5).
- Add some top-level documentation to the term API to explain some of
the basics.
Closes#1736
Minecraft sometimes keeps chunks in-memory, but not actively loaded. If
we schedule a block entity to be ticked and that chunk is is then
transitioned to this partially-loaded state, then the block entity is
never actually ticked.
This is most visible with monitors. When a monitor's contents changes,
if the monitor is not already marked as changed, we set it as changed
and schedule a tick (see ServerMonitor). However, if the tick is
dropped, we don't clear the changed flag, meaning subsequent changes
don't requeue the monitor to be ticked, and so the monitor is never
updated.
We fix this by maintaining a list of block entities whose tick was
dropped. If these block entities (or rather their owning chunk) is ever
re-loaded, then we reschedule them to be ticked.
An alternative approach here would be to add the scheduled tick directly
to the LevelChunk. However, getting hold of the LevelChunk for unloaded
blocks is quiet nasty, so I think best avoided.
Fixes#1146. Fixes#1560 - I believe the second one is a duplicate, and
I noticed too late :D.
When we remove a wired node from a network, we need to find connected
components in the rest of the graph. Typically, this requires a
traversal of the whole graph, taking O(|V| + |E|) time.
If we remove a lot of nodes at once (such as when unloading chunks),
this ends up being quadratic in the number of nodes. In some test
networks, this can take anywhere from a few seconds, to hanging the game
indefinitely.
This attempts to reduce the cases where this can happen, with a couple
of optimisations:
- Instead of constructing a new hash set of reachable nodes (requiring
multiple allocations and hash lookups), we store reachability as a
temporary field on the WiredNode.
- We abort our traversal of the graph if we can prove the graph remains
connected after removing the node.
There's definitely future work to be done here in optimising large wired
networks, but this is a good first step.
- Replace usages of WiredNetwork.connect/disconnect/remove with the
WiredNode equivalents.
- Convert "testLarge" into a proper JMH benchmark.
- Don't put a peripheral on every node in the benchmarks. This isn't
entirely representative, and means the peripheral juggling code ends
up dominating the benchmark time.
We've been out-of-date for a while now, as we needed to update
lua_menhir to work with lrgrep 3.
- Better handling of standalone names/expressions - we now correctly
handle lists of names.
- Handle missing commas in tables in a few more places.
- We checked the backing array when reading rather than the file's
length, so could read beyond the end of the file.
- We used the entry length when resizing, which effectively meant we
doubled the size of the backing array on each write.
- cc.require now uses the internal _LOADED table to get the list of
built-in globals. This fixes several globals not showing up on the
list (e.g. utf8), and allows us to inject more modules from the Java
side.
- ILuaAPI now has a getModuleName() function. This is used to inject
the API into the aforementioned _LOADED table, allowing it to be
"require"d.
One common issue we get when a program exits after handling a "key"
event is that it leaves the "char" event on the queue. This means that
the shell (or whatever program we switch in to) then receives the "char"
event, often displaying it to the screen.
Previously we've got around this by doing sleep(0) before exiting the
program. However, we also see this problem in edit's run handler script,
and I'm less comfortable doing the same hack there.
This adds a new internal discard_char function, which will either
wait one tick or return when seeing a char/key_up event.
Fixes#1705
- Mark our core test-fixtures jar as part of the "cctest", rather than
a separate library. I'm fairly sure this was actually using the
classpath version of CC rather than the legacyClasspath version!
- Add a new "testMinecraftLibrary" configuration, instead of trying to
infer it from the classpath. We have to jump through some hoops to
avoid having multiple versions of a library on the classpath at once,
but it's not too bad.
I'm working on a patch to bsl which might allow us to kill of
legacyClasspath instead. Please, anything is better than this.
Forge doesn't run client-side commands from sendUnsignedCommand, so we
still require a mixin there.
We do need to change the command name, as Fabric doesn't properly merge
the two command trees.
I didn't make a new years resolution to stop writing build tooling, but
maybe I should have.
This replaces our use of VanillaGradle with a new project,
VanillaExtract. This offers a couple of useful features for multi-loader
dev, including Parchment and Unpick support, both of which we now use in
CC:T.
- Debug hooks are now correctly called for every function.
- Fix several minor inconsistencies with debug.getinfo.
- Fix Lua tables being sized incorrectly when created from varargs.
This was copied over from the old binary handle, and so states we
always return a single number if no count is given. This is only the
case when the file is opened in binary mode.
Rather than mixing-in to CachedOutput, we just wrap our DataProviders to
use a custom CachedOutput which reformats the JSON before writing. This
allows us to drop mixins for common+non-client code.
Disk drives have had a long-standing issue with mutating their contents
on the computer thread, potentially leading to all sorts of odd bugs.
We tried to fix this by moving setDiskLabel and the mounting code to run
on the main thread. Unfortunately, this means there is a slight delay to
mounts being attached, breaking disk startup.
This commit implements an alternative solution - we now do mounting on
the computer thread again. If the disk's stack is modified, we update it
in the peripheral-facing item, but not the actual inventory. The next
time the disk drive is ticked, we then sync the two items.
This does mean that there is a fraction of a tick where the two will be
out-of-sync. This isn't ideal - it would potentially be possible to
cycle through disk ids - but I don't really think that's avoidable
without significantly complicating the IMedia API.
Fixes#1649, fixes#1686.
Originally we exposed a single registerTurtleUpgradeModellermethod which
could be called from both Fabric (during a mod's client init) and Forge
(during FMLClientSetupEvent).
This was fine until we allowed upgrades to specify model dependencies,
which would then automatically loaded, as this means model loading now
depends on upgrade modellers being loaded. Unknown to me, this is not
guaranteed to be the case on Forge - mod setup happens at the same time
as resource reloading!
Unfortunately there's not really a salvageable way of fixing this with
the current API. Forge now uses a registration event-based system,
meaning we can guarantee all modellers are loaded before models are
baked.
- Add support for version overrides/exclusions in our dependency check.
Sometimes mod loaders use different versions to vanilla, and we need
some way to handle that.
- Rescan wired network connections on the tick after invalidation,
rather than when invalidated.
- Convert some constant lambdas to static method references. Lambdas
don't allocate if they don't capture variables, so this has the same
performance and is a little less ugly.
- Small code-style/formatting changes.
Historically we used Forge's SimpleChannel methods (and
PacketDistributor) to send the packets to the client. However, we don't
need to do that - it is sufficient to convert it to a vanilla packet,
and send the packet ourselves.
Given we need to do this on Fabric, it makes sense to do this on Forge
as well. This allows us to unify (and thus simplify) a lot of how packet
sending works.
At the same time, we also remove the handling of speaker audio during
decoding. We originally did this to avoid the additional copy of audio
data. However, this doesn't work on 1.20.4 (as packets aren't
encoded/decoded on singleplayer), so it makes sense to do this
Correctly(TM).
This also allows us to get rid of ClientNetworkContext.get(). We do
still need to service load this class (as Forge's networking isn't split
up in the same way Fabric's is), but we'll be able to drop that in
1.20.4.
Finally, we move the record playing code from ClientNetworkContext to
ClientPlatformHelper. This means the network context no longer needs to
be platform-specific!
After embarrassing, let's do some proper work.
Rather than passing the level and position each time we call
ComponentAccess.get(), we now pass them at construction time (in the
form of the BE). This makes the consuming code a little cleaner, and is
required for the NeoForge changes in 1.20.4.
I was able to reproduce this by starting two computers, and then warming
up the JIT by running:
while true do os.queueEvent("x") os.pullEvent("x") end
and then running the following on one computer, while typing on the
other:
while true do end
I'm not quite sure why this happens. It's possible that once the JIT is
warm, we can resume computers without actually allocating anything,
though I'm a little unconvinced.
Fixes#1672
Everything old is new again!
CC's network message implementation has gone through several iterations:
- Originally network messages were implemented with a single class,
which held an packet id/type and and opaque blobs of data (as
string/int/byte/NBT arrays), and a big switch statement to decode and
process this data.
- In 42d3901ee3, we split the messages
into different classes all inheriting from NetworkMessage - this bit
we've stuck with ever since.
Each packet had a `getId(): int` method, which returned the
discriminator for this packet.
- However, getId() was only used when registering the packet, not when
sending, and so in ce0685c31f we
removed it, just passing in a constant integer at registration
instead.
- In 53abe5e56e, we made some relatively
minor changes to make the code more multi-loader/split-source
friendly. However, this meant when we finally came to add Fabric
support (8152f19b6e), we had to
re-implement a lot of Forge's network code.
In 1.20.4, Forge moves to a system much closer to Fabric's (and indeed,
Minecraft's own CustomPacketPayload), and so it makes sense to adapt to
that now. As such, we:
- Add a new MessageType interface. This is implemented by the
loader-specific modules, and holds whatever information is needed to
register the packet (e.g. discriminator, reader function).
- Each NetworkMessage now has a type(): MessageType<?> function. This
is used by the Fabric networking code (and for NeoForge's on 1.20.4)
instead of a class lookup.
- NetworkMessages now creates/stores these MessageType<T>s (much like
we'd do for registries), and provides getters for the
clientbound/serverbound messages. Mod initialisers then call these
getters to register packets.
- For Forge, this is relatively unchanged. For Fabric, we now
`FabricPacket`s.
- Clean up option parsing a bit, so it uses the Option, rather than its
corresponding character code.
- Add a new -L/--allow-local-domains flag to remove the $private rule
from the HTTP rules.
While ComputerFamily is still useful, there's definitely some places
where it adds an extra layer of indirection. This commit attempts to
clean up some places where we no longer need it.
- Remove ComputerFamily from AbstractComputerBlock. The only place this
was needed is in TurtleBlock, and that can be replaced with normal
Minecraft explosion resistence!
- Pass in the fuel limit to the turtle block entity, rather than
deriving it from current family.
- The turtle BERs now derive their model from the turtle's item, rather
than the turtle's family.
- When creating upgrade/overlay recipes, use the item's name, rather
than {pocket,turtle}_family. This means we can drop getFamily() from
IComputerItem (it is still needed on to handle the UI).
- We replace IComputerItem.withFamily with a method to change to a
different item of the same type. ComputerUpgradeRecipe no longer
takes a family, and instead just uses the result's item.
- Computer blocks now use the normal Block.asItem() to find their
corresponding item, rather than looking it up via family.
The above means we can remove all the family-based XyzItem.create(...)
methods, which have always felt a little ugly.
We still need ComputerFamily for a couple of things:
- Permission checks for command computers.
- Checks for mouse/colour support in ServerComputer.
- UI textures.
- Add a check to ensure declared dependencies in the :core project, and
those inherited from Minecraft are the same.
- Compute the next Cobalt version, rather than specifying it manually.
- Add the gradle versions plugin (and version catalog update), and
update some versions.
Previously we prevented our published full jar depending on any of the
other projects by excluding the whole cc.tweaked jar. However, as Cobalt
also now lives in that group, this meant we were missing the Cobalt
dependency.
Rather than specifying a wildcard, we now exclude the dependencies when
adding them to the project.
This is a bit of an odd combination of a few bugs:
- When the terminal component is blurred, we fire a mouse_up event for
the last-held button. However, we had an off-by-1 error here, so this
only triggered for the right/middle buttons.
- This was obsucuring the second bug, which is when we clicked within
the terminal, this caused the terminal to be blurred (thus releasing
the mouse) and then focused again.
We fix this by only setting the focus if there's actually a change.
Fixes#1655
When adding/removing observers, we locked on the observer, then
acquired the global lock. When a metric is observed, then we acquire the
global lock and then the observer lock.
If these happen at the same time, we can easily end up with a deadlock.
We simply avoid holding the observer lock for the entire add/remove
process (instead only locking when actually needed).
Closes#1639
This commit adds abstract classes to describe the interface for our
mod-loader-specific generic peripherals (inventories, fluid storage,
item storage).
This offers several advantages:
- Javadoc to illuaminate conversion no longer needs the Forge project
(just core and common).
- Ensures we have a consistent interface between Forge and Fabric.
Note, this does /not/ implement fluid or energy storage for Fabric. We
probably could do fluid without issue, but not something worth doing
right now.
Rather than assuming static methods are generic, and instance methods
are direct, the Generator now has separate entrypoints for handling
instance and generic methods.
As a result of this change, we've also relaxed some of the validation
code. As a result, we now allow calling private/protected methods
which are annotated with @LuaFunction.
This adds a new "java_allocation" metric, which tracks the number of
bytes allocated while executing the computer (as measured by Java). This
is not an 100% reliable number, but hopefully gives some insight into
what computers are doing.
Historically CC has supported two modes when working with file handles
(and HTTP requests):
- Text mode, which reads/write using UTF-8.
- Binary mode, which reads/writes the raw bytes.
However, this can be confusing at times. CC/Lua doesn't actually support
unicode, so any characters beyond the 0.255 range were replaced with
'?'. This meant that most of the time you were better off just using
binary mode.
This commit unifies text and binary mode - we now /always/ read the raw
bytes of the file, rather than converting to/from UTF-8. Binary mode now
only specifies whether handle.read() returns a number (and .write(123)
writes a byte rather than coercing to a string).
- Refactor the entire handle hierarchy. We now have an AbstractMount
base class, which has the concrete implementation of all methods. The
public-facing classes then re-export these methods by annotating
them with @LuaFunction.
These implementations are based on the
Binary{Readable,Writable}Handle classes. The Encoded{..}Handle
versions are now entirely removed.
- As we no longer need to use BufferedReader/BufferedWriter, we can
remove quite a lot of logic in Filesystem to handle wrapping
closeable objects.
- Add a new WritableMount.openFile method, which generalises
openForWrite/openForAppend to accept OpenOptions. This allows us to
support update mode (r+, w+) in fs.open.
- fs.open now uses the new handle types, and supports update (r+, w+)
mode.
- http.request now uses the new readable handle type. We no longer
encode the request body to UTF-8, nor decode the response from UTF-8.
- Websockets now return text frame's contents directly, rather than
converting it from UTF-8. Sending text frames now attempts to treat
the passed string as UTF-8, rather than treating it as latin1.
- Update Cobalt to 0.8.0, switching our Lua version to 5.2(ish).
- Remove our `load` wrapper, as we no longer need to inject _ENV into
the enviroment table.
- Update the parser to handle labels and goto. This doesn't check that
gotos are well formed, but at least means the parser doesn't fall
over on them.
- Update our docs to reflect the changes to Cobalt.
This tries to cover some holes in our existing coverage.
- Port some of our Java readable handle tests to Lua (and also clean up
the Java versions to stop using ObjectWrapper - that dates to
pre-@LuaFunction!)
- Test a couple of discrepancies between binary and text handles. This
is mostly to do with the original number-based .read() and .write()
interface for binary handles.
- Fix a couple of edge cases in file-size accounting.
Does it count as an emulator when it's official? I hope not, as this'd
make it my fourth or fifth emulator at this point.
* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
Developing/debugging CraftOS is a massive pain to do inside Minecraft,
as any change to resources requires a compile+hot swap cycle (and
sometimes a `/reload` in-game). As such, it's often more convenient to
spin up an emulator, pointing it to load the ROM from CC:T's sources.
However, this isn't practical when also making changes to the Java
classes. In this case, we either need to go in-game, or build a custom
version of CCEmuX.
This commit offers an alternative option: we now have our own emulator,
which allows us to hot swap both Lua and Java to our heart's content.
Most of the code here is based on our monitor TBO renderer. We probably
could share some more of this, but there's not really a good place for
it - feels a bit weird just to chuck it in :core.
This is *not* a general-purpose emulator. It's limited in a lot of
ways (won't launch on Mac[^1], no support for multiple computers) - just
stick to what's there already.
[^1]: We require OpenGL 4.5 due to our use of DSA.
In practice, we're never going to change this to true by default. The
old Tekkit Legends pack enabled this[^1], and that caused a lot of
problems, though admittedly back in 2016 so things might be better now.
If people do want this functionality, it should be fairly easy to
replicate with a datapack, adding a file to rom/autorun.
[^1]: See https://www.computercraft.info/forums2/index.php?/topic/27663-
Hate that I remember this, why is this still in my brain?
- Move most error message constants to a new MountHelpers class.
- Be a little more consistent in when we throw "No such file" vs "Not a
file/directory" messages.
The two implementations aren't entirely compatible - the implementation
returned by .of will throw an NPE on .contains(null), whereas the
Collections implementations just return false. However, we try to avoid
passing null to collections methods, so this should be safe.
There's no strong reason to do this, but it helps make the code a little
more consistent
As of 1.20, sign messages are immutable - we need to do
text = text.setMesssage(...) instead. Also do a tiny bit of cleanup to
this function while we're here.
Probably not the best use of my lunch break :D:.
Fixes#1611.
This is an attempt to enforce better separation between ComputerThread
and ComputerExecutor. Both of these classes are pretty complex in their
own right, and the way the two bleed into each other makes it all the
more confusing!
This effectively splits the ComputerExecutor into two separate classes:
- ComputerScheduler.Executor (with the actual implementation inside
ComputerThread): This holds all the ComputerThread-related logic
which used to be in ComputerExecutor, including:
- before/after work hooks
- is-on-thread tracking
- virtual runtime computation
- ComputerScheduler.Worker: This encapsulates all the computer-related
behaviour. The actual implementation remains in ComputerExecutor.
The boundaries are still a little fuzzy here, and it's all definitely
more coupled then I'd like, but still an improvement!
There are several additional changes at the same time:
- TimeoutState has also been split up, to better define the boundary
between consumers (such as ComputerExecutor and ILuaMachine) and
controllers (ComputerThread).
The getters still live in TimeoutState, but the core logic lives in
ManagedTimeoutState.
- We no longer track cumulative time in the TimeoutState. Instead, we
allow varying the timeout of a computer. When a computer is paused,
we store the remaining time, and restore it when resuming again.
This also allows us give a longer timeout for computer
startup/shutdown, hopefully avoiding some of those class-not-found
issues we've seen.
- We try to make the state machine of how ComputerExecutors live on the
queue a little more explicit. This is very messy/confusing -
something I want to property test in the future.
I'm sure there's more to be done here, especially in ComputerExecutor,
but hopefully this makes future changes a little less intimidating.
Allows registering arbitrary block lookup functions instead of a
platform-specific capability. This is roughly what Fabric did before,
but generalised to also take an invalidation callback.
This callback is a little nasty - it needs to be a NonNullableConsumer
on Forge, but that class isn't available on Fabric. For now, we make the
lookup function (and thus the generic peripheral provider) generic on
some <T extends Runnable> type, then specialise that on the Forge side.
Hopefully we can clean this up when NeoForge reworks capabilities.
This is the second time I've rewritten our class generation in a little
over a month. Oh dear!
Back in d562a051c7 we started using method
handles inside our generated ASM, effectively replacing a direct call
with .invokeExact on a constant method handle.
This goes one step further and removes our ASM entirely, building up a
MethodHandle that checks arguments and then wraps the return value.
Rather than generating a class, we just return a new LuaFunction
instance that invokeExacts the method handle.
This is definitely slower than what we had before, but in the order of
8ns vs 12ns (in the worst case, sometimes they're much more comparable),
so I'm not too worried in practice.
However, generation of the actual method is now a bit faster. I've not
done any proper benchmarking, but it's about 20-30% faster.
This also gives us a bit more flexibility in the future, for instance
uisng bound MethodHandles in generation (e.g. for instance methods on
GenericSources). Not something I'm planning on doing right now, but is
an option.
