Skip to content

New event verbose command#9024

Closed
TheRealRyGuy wants to merge 13 commits into
PaperMC:masterfrom
TheRealRyGuy:feature/event-verbose
Closed

New event verbose command#9024
TheRealRyGuy wants to merge 13 commits into
PaperMC:masterfrom
TheRealRyGuy:feature/event-verbose

Conversation

@TheRealRyGuy
Copy link
Copy Markdown
Contributor

Saw a message from Machine Maker on Discord bringing this up, figured it'd be a fun random thing to do

Essentially just adds a /paper dumpevents command. Running this once will begin logging events, and running it again will save the events in order with timestamp and event class fields in JSON to a file in the debug directory. Attached below is an example output

Two issues that aren't too big of a deal with this

Does not handle inheritance of event classes well, Field#getDeclaredFields only retrieves minimal fields, could use a superclass check working all the way up to org.bukkit.Event
Could further serialize non primitive event fields just for a bit more info, stringified versions of them are typically fine though

event-verbose-2023-03-21_18.03.40.txt

@TheRealRyGuy TheRealRyGuy requested a review from a team as a code owner March 21, 2023 22:33
@Machine-Maker
Copy link
Copy Markdown
Member

Machine-Maker commented Mar 21, 2023

Part of what I had described was logging to be able to see event class fields before and after each listener for an event. That way if an an event had 50 listeners, you could try and find what listener changed what.

EDIT: Another thing, is I feel some events should be excluded by default. Like the tick events, ones that are just spammed a ton. Can have something that does include those, but most of the time they are just gonna clog up the system. We can also exclude events who don't have "changeable" values.

@TheRealRyGuy
Copy link
Copy Markdown
Contributor Author

Most QoL way to do that would be with an EventExecutor and listen for a specific event - not sure if there's a way to register one internally though. My thought process was that this was just for all of the "what event runs when xyz" questions pop up.

Will exclude tick start and end events, not sure what you mean by "changeable" values though.

@Machine-Maker
Copy link
Copy Markdown
Member

Machine-Maker commented Mar 21, 2023

In the SimplePluginManager#fireEvent and PaperEventManager#callEvent, can't you just get the values of the event before and after each call to the listeners?

"changeable" is hard to define. Generally I think events that have all final fields with no mutable things exposed don't need to be included. These events are just events used to inform that something happened, and a listener can't actually change anything about the event. The VehicleUpdateEvent is one of those. You can't change anythinga bout the event, so it doesn't need to be logged. Although I'm not sure there's a good way of knowing this automatically.

Note: I had to do this in a skewed way cause my git is losing its mind, if this broke something my fault
@TheRealRyGuy
Copy link
Copy Markdown
Contributor Author

Now ignores all final fields and can take specific exclusions, right now just tick starting and ending

Worth keeping this as its own system? Can just make a new command for watching specific events, can also just nuke this feature entirely for what you were more thinking of

@Machine-Maker
Copy link
Copy Markdown
Member

I would exclude final collections (Collection, Map) and ItemStack from the final check. It's possible to have a final one of those that can be mutated via a getter.

I suppose both are useful. Seeing what events fire between two points, and a deeper look at what changes for each listener for those events. Might be best for the system I imagined to specify a single event to do that for.

@TheRealRyGuy
Copy link
Copy Markdown
Contributor Author

TheRealRyGuy commented Mar 22, 2023

Added in the general premise for what you were looking for through /paper watchevent [event], attached an example output that's just one listener modifying async chat's format.

Also moved the event verbose to paper's event manager to ignore bad event calls. The final check of that also now excludes org.bukkit.inventory.ItemStack as well as collections and maps, however, if we're just looking for the general flow of things, don't we want to just include every event?

event-AsyncPlayerChatEvent-2023-03-21_20.43.03.txt

Comment thread patches/server/0970-New-specific-event-watching-command.patch Outdated
Comment thread patches/server/0969-New-event-verbose-logging-command.patch Outdated
Comment thread patches/server/0969-New-event-verbose-logging-command.patch Outdated
Comment thread patches/server/0969-New-event-verbose-logging-command.patch Outdated
Comment thread patches/server/0969-New-event-verbose-logging-command.patch Outdated
Comment thread patches/server/0970-New-specific-event-watching-command.patch Outdated
Comment thread patches/server/0970-New-specific-event-watching-command.patch Outdated
Comment thread patches/server/0970-New-specific-event-watching-command.patch Outdated
Comment thread patches/server/0970-New-specific-event-watching-command.patch Outdated
Comment thread patches/server/0970-New-specific-event-watching-command.patch Outdated
@TheRealRyGuy
Copy link
Copy Markdown
Contributor Author

All should be done unless I missed anything
dumpevents now saves to the event-debug folder, whereas watchevent will log into event-debug/[event name]/ dir

Comment thread patches/server/0969-New-specific-event-watching-command.patch Outdated
Comment thread patches/server/0969-New-specific-event-watching-command.patch Outdated
Comment thread patches/server/0969-New-specific-event-watching-command.patch Outdated
Comment thread patches/server/0969-New-specific-event-watching-command.patch Outdated
Comment thread patches/server/0968-New-event-verbose-logging-command.patch
Comment thread patches/server/0969-New-specific-event-watching-command.patch Outdated
Copy link
Copy Markdown
Member

@Owen1212055 Owen1212055 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally, look at your field names, some of them are UPPERCASE_EXAMPLE when not needed.

Comment thread patches/server/0968-New-event-verbose-logging-command.patch Outdated
Comment thread patches/server/0969-New-specific-event-watching-command.patch Outdated
Comment thread patches/server/0968-New-event-verbose-logging-command.patch Outdated
Comment thread patches/server/0969-New-specific-event-watching-command.patch Outdated
Comment thread patches/server/0968-New-event-verbose-logging-command.patch
Comment thread patches/server/0968-New-event-verbose-logging-command.patch Outdated
+ };
+ }
+
+ private void dumpToFile(CommandSender sender, String event, List<Wrapper> wrappers) throws IOException {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general, events can quickly accumulate. I think it may be better to instead actively log to a file async, similar to the latest.log that is auto generated. Have it log an event instead of having to store all of these event calls in memory.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is latest.log not just sl4j? Would appreciate a point towards an async internal task method

Comment thread patches/server/0968-New-event-verbose-logging-command.patch Outdated
@Warriorrrr Warriorrrr moved this from Awaiting review to Waiting For Author in Paper PR Queue Mar 5, 2025
@kennytv kennytv added the pre-softspoon: never rebased Pre-hardfork pull requests that were not re-opened with the new main branch label Mar 23, 2025
@kennytv kennytv deleted the branch PaperMC:master March 23, 2025 19:15
@kennytv kennytv closed this Mar 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pre-softspoon: never rebased Pre-hardfork pull requests that were not re-opened with the new main branch pre-softspoon

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

7 participants