Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rejig LogStringHandler #30706

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Conversation

ElectroJr
Copy link
Member

@ElectroJr ElectroJr commented Aug 6, 2024

This PR reworks parts of admin logging and LogStringHandler, both to make it slightly nicer to use, and to remove some unnecessary component queries. The main change is that a lot of the processing that was happening in AdminLogManager.ToJson() is now done in the string interpolation handler (LogStringHandler).

Also fixes #31531

Changes:

  • The handler now automatically uses ToPrettyString() when given an entity. I.e., $"{ToPrettyString(user):user} interacted with {ToPrettyString(target):target}" can now be replaced with $"{user} interacted with {target}".
  • The handler now recognises NetEntity and objects that implement IAsType<EntityUid> as entities. This means that Entity<T1,T2..> should be recognized as entities once they are given the IAsType interface (which is included in Toolshed Rejig RobustToolbox#5455)
  • Components no longer work as references to an entity (Component.Owner is obsolete).
  • Changed some json serializers:
    • Removed SerializableEntityCoordinates and replaced the json converter with something that just directly uses EntityCoordinates. I'm not really sure why this struct even existed.
    • The SerializablePlayer struct was moved from a sever-side to a shared namespace
    • I'm not sure how the admin log tools work, so I'm not sure if these changes could somehow causes issues

@github-actions github-actions bot added Status: Needs Review This PR requires new reviews before it can be merged. labels Aug 6, 2024
@ElectroJr ElectroJr added the Needs Engine PR This PR is awaiting engine changes that should be merged before this. label Aug 7, 2024
@github-actions github-actions bot added the Merge Conflict This PR currently has conflicts that need to be addressed. label Sep 2, 2024
Copy link
Contributor

github-actions bot commented Sep 2, 2024

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@ElectroJr ElectroJr added Needs #codebase-changes This PR has changes that should be announced in the #codebase-changes discord channel. and removed Needs Engine PR This PR is awaiting engine changes that should be merged before this. labels Sep 6, 2024
@github-actions github-actions bot removed the Merge Conflict This PR currently has conflicts that need to be addressed. label Sep 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs #codebase-changes This PR has changes that should be announced in the #codebase-changes discord channel. Status: Needs Review This PR requires new reviews before it can be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Admin log issue, MindOwnerLoggingString()
1 participant