-
Notifications
You must be signed in to change notification settings - Fork 12
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
Moved several commands from RPGMods #10
base: main
Are you sure you want to change the base?
Conversation
I remember you saying you didn't like having all that in the helper class, so the find and kick utilities for the kick command are in here. the moved commands are: Kick, Shutdown, and Ping in the new Server Commands file, and List All Items in the Search Commands
foreach (var entry in gameDataSystem.ItemHashLookupMap) { | ||
try { | ||
var item = managed.GetOrDefault<ManagedItemData>(entry.Key); | ||
ctx.Reply(item.PrefabName); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a lot of messages to send. I'd prefer for performance that it send fewer messages with newlines. I'm personally not sure that we should have both search and list items. It makes me think there's improvements that the search needs if it's not showing enough information.
try { | ||
var item = managed.GetOrDefault<ManagedItemData>(entry.Key); | ||
ctx.Reply(item.PrefabName); | ||
Core.Log.LogInfo("Prefab Name: " + item.PrefabName); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this needs to be logged with each call.
ctx.Reply(item.PrefabName); | ||
Core.Log.LogInfo("Prefab Name: " + item.PrefabName); | ||
|
||
} catch { } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you know what throws here? Should it not be logged?
public class ServerCommands { | ||
public static class Shutdown { | ||
[Command("shutdown", shortHand: "quit", description: "Trigger the exit signal & shutdown the server.", adminOnly: true)] | ||
public static void Initialize() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this work? VCF commands should require ICommandContext
} | ||
} | ||
|
||
public static class Kick { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please reference the existing code in this project for examples. You should utilize the existing converters and systems for finding players for example.
} | ||
} | ||
|
||
public static class Ping { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As above remove wrapper class, please format code.
I remember you saying you didn't like having all that in the helper class, so the find and kick utilities for the kick command are in here. the moved commands are: Kick, Shutdown, and Ping in the new Server Commands file, and List All Items in the Search Commands. My intention is to move the general utilities to here.