Vanilla-ish commands#67
Conversation
ReCore-sys
added
enhancement
|
I haven't taken a definitive look to be able to give a review or point out things, but this generally looks like a huge improvement to the old system |
MSKatKing
left a comment
MSKatKing
left a comment
There was a problem hiding this comment.
Overall it looks really good! I just had a couple questions and one concern, so if you wouldn't mind taking a look at those that would amazing!
|
|
||
| #[derive(Clone, Debug, Eq, PartialEq)] | ||
| pub struct PositionArg { | ||
| pub x: String, |
There was a problem hiding this comment.
Is there any reason this should be stored as a String? Could it be better to store it as a separate struct or tuple that has the f64 coord and a flag for if it's relative?
There was a problem hiding this comment.
I had it this way so the relative-ness and parsing could all be handled in one function, but I don't think it makes all that might difference in the end.
| .is_ok_and(|player_permissions| player_permissions.can(permission)), | ||
| } | ||
| }; | ||
| if let Some(permission) = C::permission() |
There was a problem hiding this comment.
If permissions are checked here then why do some handlers check it themselves? Should a precondition for the handler function be stated as the CommandSource having the necessary permissions?
There was a problem hiding this comment.
Look I'm not going to lie, I forgot I added this by the time I got around to making the commands.
3 participants
After a stupid amount of time losing my mind over macro code, here's some kinda impressive command nonsense.
Description
Replaced the entire command system (sorry @radstevee) with a new macro and enum based system. It should hopefully be as simple as possible to create new commands from just an enum/struct, a macro and filling out a trait. It is able to replicate the vanilla command system pretty much entirely so with the exception of some of the more complicated commands (/execute im looking at you) we should be able to get vanilla compatible commands.
Motivation and Context
The old system had a lot of
problemspersonality, but it was pretty limited and kinda tricky to use from a developer standpoint. This new system is fairly simple to use and should be considerably better.How has this been tested?
Bunch of unit tests and in-game testing.
Screenshots (if appropriate):
Types of changes
Checklist: