Event unification and touch damage by IntegratedQuantum · Pull Request #2144 · PixelGuys/Cubyz

IntegratedQuantum · 2025-10-27T16:41:15Z

Unifies gui opening, tick events and on touch events to use the same system.

This makes it more future proof and establishes a clear pattern to be used for events.

Also while I was at it I decided to implement lava damage.

Implement damage for more blocks: Cactus and magma

brings us one step closer to #2083

… the shift key

IntegratedQuantum · 2025-10-27T17:12:34Z

@Argmaster I would like to hear your opinion on this. Is this in line with what you wanted to have in #2083?

Argmaster

@Argmaster I would like to hear your opinion on this. Is this in line with what you wanted to have in #2083?

Partially, yes. I did a rough review for now and I have some thoughts, but I would want to make clear what is a simplification to avoid doing to much at once and what is a design choice. So, are we aiming to have full event loop setup, or are you planning to stop with what we have now (basically callbacks but more consistent)?

src/game.zig

Argmaster · 2025-10-27T20:18:18Z

src/game.zig

 	}

-	pub fn touchBlocks(entity: main.server.Entity, hitBox: Box, side: main.utils.Side) void {
+	pub fn touchBlocks(entity: *main.server.Entity, hitBox: Box, side: main.utils.Side, deltaTime: f64) void {


So, a touch happens when you collide with block hitbox, hm?
I "touch" immediately brings "touch with your finger" instead of "touch with your face" idk about you.

The sense of touch is present on your whole body.
For colliding into things (as in moving towards them) I would like to have separate functions. This could in theory also allow to move some physics effects like bouncing into the event handler.

Hm, I guess I will let you cook.

src/events/block/client/openWindow.zig

Argmaster · 2025-10-27T20:32:51Z

src/events/block/server/replaceBlock.zig

+	return result;
+}
+
+pub fn run(self: *@This(), params: main.events.ServerBlockEvent.Params) main.events.EventResult {


How is this event supposed to work? When I am allowed to use it? Can I use it in WE commands?

You can use it in any context that uses a ServerBlockEvent, currently it's only used in random tick events, but you could make a world edit command that triggers a ServerBlockEvent based on a user-provided zon if you think that's useful.

Argmaster · 2025-10-27T21:51:49Z

src/callbacks/block/server/replaceBlock.zig

Is there any other replace event in blocks category? Why not replace.zig

I could see a replace callback for bigger areas, like spheres or cubes. Like maybe you have a corruption block and want to spread it in all directions on random tick.

Ok, its a valid argument.

Argmaster · 2025-10-27T21:53:51Z

src/events/events.zig

+fn GenericEvent(_Params: type, list: type) type {
+	return struct {
+		data: *anyopaque,
+		runFunction: *const fn(self: *anyopaque, params: Params) main.events.EventResult,


There is a well established name for such things: callback

Isn't callback generally the name of the entire object that holds the function pointer? At least that's how it was in java.

I guess depends on imagination of the author

In this case I would prefer to stick to definition that pops up if you DuckDuckGo "what is a callback", in my case:

So it is easier for uninformed user to learn.
DuckDuckGoing runFunction doesn't yield useful results.

How would I call run then? It would seem inconsistent to have it called run in all but this one place.

Well, with current implementation it is no longer an event, more of an event handler according to definition on Wikipedia. Event is just a piece of data without callback. If we rename this to event handler then run as a function to trigger it makes sense. Additionally I don't see a problem with runing a callback as an interpretation of what is happening.

I'm wondering if maybe we should call the whole thing a Callback then. It's conceptually no different from a lambda callback that also captures extra parameters.

src/events/events.zig

src/events/block/touch/hurt.zig

IntegratedQuantum · 2025-10-28T08:15:33Z

So, are we aiming to have full event loop setup, or are you planning to stop with what we have now (basically callbacks but more consistent)?

I'm not sure yet. I would like to avoid an event loop, since it does have a performance overhead (→needs to allocate and store the event data in a data structure), but we might need it anyways.
Either way with a unified callback system it should be easy to add that on top later, by just changing the parameter type to a pointer.

Argmaster · 2025-10-28T15:21:15Z

Ok, then I need to gather my thoughts and write them down here, I will do that after work.

Argmaster · 2025-10-28T16:47:35Z

src/events/events.zig

+			.runFunction = &ignoredRun,
+		};
+
+		fn ignoredRun(_: *anyopaque, _: Params) EventResult {


Is that noop with a fancy name?

Argmaster

Ok, so this is quite different than the direction #2083 is proposing.

TL;DR; Event loop please, so callback is always called the same way.

Current Event is "just" an implementation of a stateful lambda. It unifies the interface of callbacks for blocks, which is great, they really needed that, but at the same time has typical callback issues - those callbacks have no guarantees where they will be called from (as in, which thread they are called from). Thus callee can never be sure what it can and what it cannot do. I best case scenario this means a bunch more mutex locks and unlocks, in worst case it means we won't be able to reuse that code, as depending on context necessary locks exclude each other.
Consider following case: You have an entity in ECS that can place or break a block (eg. upon death). Can you call place block from entity AI thread? Why does entity AI thread lock a chunk mutex? Even worse, why entity AI code has to bother with which replace block to call (since you have separation between server and client implementation). Entity AI would be happy if it would just schedule a block placement and would wait until that blocks appears where it asked it to appear after some time, checking for it every tick.
Same applies to playing sounds, spawning particles etc. Why does X care about action being immediate? Delaying it for 1, 5, 10 frames doesn't bother anyone, even better, it may allow us to optimize things automagically if events can be batched into single thing. We are not talking about actions in a tight loop in the middle of the rendering thread, we are talking about things that happen at human perception speed.

TL;DR; Events and Listeners as a separate system from Block system and storage.

Now back to genericness of the design. Current solution is closely coupled with block system for no reason at all. An alternative implementation would be to define Events to contain no static data (not zon configurable, only Params in current implementation terms) and Listeners to be configured with necessary zon configuration.
Regardless of looped or loop-less implementation, when an event is triggered it would be delivered to a listener dedicated to that event and said listener would check which block was delivered to it and act accordingly. Block would no longer know that it opens a GUI. Instead you would define listener as code and include a zon configuration for it that would contain all the necessary information for it to function, in format preferred by the listener implementation.
This means we can finally stop adding more and more complexity to blocks.zig and distribute it between smaller systems with smaller interfaces. My experience shows that spaghetti factor of a module grows exponentially with its size.

Final notes:

what you are implementing is different design pattern called Command (as opposed to Observer I have suggested)
I am far from saying PR is all bad, although my comments may have suggest otherwise. In current state its still a step in a good direction, but I think its too small of a step.
I am convinced that this is not performance critical code and it should value readability and extensibility over performance. If someone is dedicated enough, this will always be a weak spot, for one reason or another (eg. inventory lock). I do not think we should throw performance out the window with O(n!) complexity, but we should split the responsibilities between different systems and in that case make the system as generic as possible. To my best knowledge it it possible to generalize it so its not directly tied to any of existing systems.
I did recently listen to non-pessimisation and "Clean" Code, Horrible Performance and I do see he has a point. There are two "but" I would like point out tho:
- this is just a guideline. Guidelines should be always tested against priorities of the project and reality. Here reality is that for 99% of the time opening a chest or touching a cactus is not a performance critical path in the code (or rather it shouldn't be!)
- There is a big difference between using "clean code" practices at high level, when managing big systems and communication between them (like here) where overhead from the cleanness of the code is in single digits of execution time, compared to using vtables in the middle of rendering thread. Again, guidelines should be tested against reality, not followed blindly.

IntegratedQuantum · 2025-10-28T19:31:38Z

I think you are underestimating the performance impact of this. Yes touching a cactus is rare, but if we have a thousand entities touching the ground, and 10 different listeners that listen to this on-touch event, then that would be quite expensive.
The same is also true for random tick events, we want to process 10000s of these, and if each of them first needs to go through 10 different listeners which all come to the conclusion that no action is needed, then this is a lot of wasted processing power.

I agree that we should be careful about what context an event gets executed.
For that I would like to have assertion for what thread you are one, and if we ever do get into trouble then we can just put events into a queue and execute them elsewhere, like how it's already done for e.g. window open/close operations, client block updates from the server, and the particle command.
There are already events that are executed in a clean context (→random tick) and shouldn't have this extra overhead.

This means we can finally stop adding more and more complexity to blocks.zig and distribute it between smaller systems with smaller interfaces.

I do think the block type is the right place to put block-type specific event data. In my opinion the block should know what window it opens, and the user should be able to specify that in the block's zon file.
I do not see how this adds more complexity to blocks.zig, it's all rather trivial code in there (some simple parsing, some member functions that return an array value, some arrays).

Argmaster · 2025-10-28T19:51:10Z

Then I suppose I have nothing else to add.

Argmaster · 2025-10-29T22:37:11Z

src/zon.zig

 	pub fn getChild(self: *const ZonElement, key: []const u8) ZonElement {
-		if(self.* != .object) {
-			return .null;
-		} else {
+		if(self.* == .object) {
+			if(self.object.get(key)) |elem| {
+				return elem;
+			}
+		}
+		return .null;
+	}
+
+	pub fn getChildOrNull(self: *const ZonElement, key: []const u8) ?ZonElement {
+		if(self.* == .object) {
 			if(self.object.get(key)) |elem| {
 				return elem;
-			} else {
-				return .null;
 			}
 		}
+		return null;
 	}


Suggested change

pub fn getChild(self: *const ZonElement, key: []const u8) ZonElement {

if(self.* != .object) {

return .null;

} else {

if(self.* == .object) {

if(self.object.get(key)) |elem| {

return elem;

}

}

return .null;

}

pub fn getChildOrNull(self: *const ZonElement, key: []const u8) ?ZonElement {

if(self.* == .object) {

if(self.object.get(key)) |elem| {

return elem;

} else {

return .null;

}

}

return null;

}

pub fn getChild(self: *const ZonElement, key: []const u8) ZonElement {

return self.getChildOrNull(key) orelse .null;

}

pub fn getChildOrNull(self: *const ZonElement, key: []const u8) ?ZonElement {

if(self.* == .object) {

return self.object.get(key) orelse .null;

}

return null;

}

src/callbacks/callbacks.zig

Argmaster · 2025-10-30T14:50:21Z

src/callbacks/block/touch/hurt.zig

+}
+
+pub fn run(self: *@This(), params: main.events.BlockTouchCallback.Params) main.events.EventResult {
+	std.debug.assert(params.entity == &main.game.Player.super); // TODO: Implement on the server side


Todos are forbidden. Make an issue.
I think that for documentation purposes it would be appropriate to link it here in a comment, but that depends on the preference.

I repurposed #1124 for this.

Argmaster

Based on my current state of knowledge it should be good to merge.

One thing to consider is having a thread local variable that identifies threads as client side and server side, so we can assert that things like server callbacks and client callbacks are invoked only on threads they are allowed to be invoked on.

Argmaster · 2025-10-31T14:00:17Z

src/callbacks/callbacks.zig

+fn Callback(_Params: type, list: type) type {
+	return struct {
+		data: *anyopaque,
+		runFunction: *const fn(self: *anyopaque, params: Params) Result,


I see runFunction persists. I suspect that is your preference, and the decision is final.
If that is the case, I would want to make
a poll on discord to see what ideas wider audience would have (for scientific purposes ofc).

Given code snippet:

fn Callback(_Params: type, list: type) type { return struct { data: *anyopaque, <field-name>: *const fn(self: *anyopaque, params: Params) Result, ...

What field name would you chose for <field-name>

go make the poll, we can wait a bit longer on this,

Unifies gui opening, tick events and on touch events to use the same system. This makes it more future proof and establishes a clear pattern to be used for events. Also while I was at it I decided to implement lava damage. - [x] Implement damage for more blocks: Cactus and magma brings us one step closer to PixelGuys#2083

IntegratedQuantum added 9 commits October 27, 2025 17:32

Pass keyboard modifiers to all press and release functions and remove…

68b66bf

… the shift key

Create a folder for events

255e557

Create BlockInteract event

7196a43

Create an openWindow event that is used in the workbench

8e410d3

Rename to just BlockEvent

e5740ad

Rename to ClientBlockEvent

f17e111

Create ServerBlockEvents to be used in tick events

9f993b4

Rename to onTick

04acd89

Move touch events to the new thing and deal damage when touching lava

38b2948

This was referenced Oct 27, 2025

Add item use effects #1939

Closed

Add replaceWith tick event #1476

Closed

Add decay tick event #1508

Closed

Of course I forgot to commit the files

c49fa06

IntegratedQuantum added 2 commits October 27, 2025 19:04

formatting

fa47a9e

Cactus hurts

6710213

Argmaster suggested changes Oct 27, 2025

View reviewed changes

thecreare reviewed Oct 28, 2025

View reviewed changes

src/events/block/touch/hurt.zig Outdated Show resolved Hide resolved

IntegratedQuantum added 4 commits October 28, 2025 09:35

Just Event

4b8abd9

Consistently call it init

8ffb32c

Uknown

8946f59

isNoop

1b1e3e6

IntegratedQuantum requested a review from Argmaster October 28, 2025 08:41

Argmaster reviewed Oct 28, 2025

View reviewed changes

Noopification

5f16abd

IntegratedQuantum requested a review from Argmaster October 29, 2025 15:19

Argmaster reviewed Oct 29, 2025

View reviewed changes

IntegratedQuantum mentioned this pull request Oct 30, 2025

Add clickThrough property to GuiWindow #2164

Closed

IntegratedQuantum added 2 commits October 30, 2025 13:20

Event→Callback

0aa337e

Simplify zon getChild functions

43ab09d

IntegratedQuantum requested a review from Argmaster October 30, 2025 12:22

IntegratedQuantum mentioned this pull request Oct 30, 2025

Patches to allow for rope placement/breaking #2077

Closed

Argmaster reviewed Oct 30, 2025

View reviewed changes

Un-event-ful renaming

090cd53

IntegratedQuantum requested a review from Argmaster October 30, 2025 20:12

Argmaster approved these changes Oct 31, 2025

View reviewed changes

IntegratedQuantum changed the title ~~Event unification~~ Event unification and touch damage Nov 1, 2025

Inner

2eedd60

IntegratedQuantum merged commit dda5fcb into PixelGuys:master Nov 1, 2025
1 check passed

IntegratedQuantum mentioned this pull request Feb 2, 2026

Change block interaction override to crouch keybind instead of shift #2531

Open

Conversation

IntegratedQuantum commented Oct 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

IntegratedQuantum commented Oct 27, 2025

Uh oh!

Argmaster left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

IntegratedQuantum commented Oct 28, 2025

Uh oh!

Argmaster commented Oct 28, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Argmaster left a comment

Choose a reason for hiding this comment

Uh oh!

IntegratedQuantum commented Oct 28, 2025

Uh oh!

Argmaster commented Oct 28, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Argmaster left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

IntegratedQuantum commented Oct 27, 2025 •

edited

Loading