Skip to content

feat: batched async logging and in-game log viewer - #6

Merged
Mitality merged 12 commits into
masterfrom
logging
Jul 17, 2026
Merged

feat: batched async logging and in-game log viewer#6
Mitality merged 12 commits into
masterfrom
logging

Conversation

@Mitality

Copy link
Copy Markdown
Contributor
image

@Mitality
Mitality requested a review from Jsinco July 12, 2026 20:27

@Jsinco Jsinco left a comment

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.

Looks good for the most part, this is just a quick glance. I'll do a more thorough review later tonight or tomorrow morning after the minor stuff i just marked gets addressed

Comment thread src/main/java/dev/jsinco/malts/commands/subcommands/LogsCommand.java Outdated
Comment thread src/main/java/dev/jsinco/malts/commands/subcommands/MaxCommand.java
Comment thread src/main/java/dev/jsinco/malts/logging/ItemLogFormatter.java

@Jsinco Jsinco left a comment

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.

Hopefully that's not too much stuff

Comment thread src/main/java/dev/jsinco/malts/events/VaultListener.java Outdated
Comment thread src/main/java/dev/jsinco/malts/gui/EditVaultGui.java Outdated
return;
}
boolean add = action == LogAction.WAREHOUSE_ADD;
String verb = add ? "added" : "removed";

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.

I would avoid hardcoding these because Malts supports multiple languages

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.

But the logger doesn't..?

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.

This should use specific keys from the lang then in order to preserve i18n around Malts. If you're not up to that then I'm still fine with pulling as-is, just mark // TODO: lang around here

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.

Well, the logs don't store any formatting information, so if we allow this to be customized for languages with different sentence structures (e.g. German), it'll make it borderline impossible to parse the logs with regex like we currently do. The only realistic way I see to make the logs fully translatable would be to switch them to something to JSON that's able to store more info, but is less human-readable. But I figured that logs would be a part of the plugin that doesn't need to be and typically isn't translated, just like the primary config file. In my experience, translations are used for whatever regular players see in game

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.

Or we could technically add a translation step between parsing the English logs and showing them to the player in game

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.

Just mark a TODO at the top of malts logger or the class storing regex noting a condensed version of that for future reference then

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.

But that would also require much more intelligent parsing

@Mitality
Mitality requested a review from Jsinco July 15, 2026 23:41

@Jsinco Jsinco left a comment

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.

LGTM other than the TODO note

@Mitality
Mitality merged commit 7fb1075 into master Jul 17, 2026
2 checks passed
@Mitality
Mitality deleted the logging branch July 17, 2026 20:09
@Mitality
Mitality restored the logging branch July 18, 2026 02:58
@Mitality
Mitality deleted the logging branch July 18, 2026 03:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants