NebulaModTeam / nebula

A multiplayer mod for the game Dyson Sphere Program
GNU General Public License v3.0
835 stars 124 forks source link

Implement gift command #640

Open mmjr-x opened 8 months ago

mmjr-x commented 8 months ago

Notable changes:

TODO:

PhantomGamers commented 8 months ago

I know this is still marked as a draft but just to give my 2 cents with that latest commit, I don't love the idea of the command requiring a user id instead of a username.

I haven't done any work with these chat commands in the past but if usernames are not guaranteed to be unique, what happens with the whisper command if there are multiple people with the same username? Because that command does just take in a username and I think that's more intuitive for players to use, though I do fully support userid being an option.

I think maybe the best approach is to have username be the main method of sending things, and then if there are multiple people with the same name then when you execute the command the game would tell you there are multiple players with the same name and list out the ids of those players and ask the sender to specify which they were referring to.

Also smaller thing that I'm not sure if anyone else agrees with but IMO this command shouldn't be /gift but like /give or /giveitem or something like that.

mmjr-x commented 8 months ago

@PhantomGamers How I implemented it now is that the command both works with userId or username (you can use either) if that is not the case at the moment then I made a mistake as it should definitely support both (and I will fix that), does that satisfy your request or is it more about the internals?

I am not sure what happens with the whisper command although I expect it just picks the first player matching that username in the list. Now that I think about It my code does the same if you provide a username that is shared by multiple users. I will make it so that it will block the gift in that case and will notify you to use the userId instead with the userIds for that username which are available to your, I think this falls in line with what you suggest?

I'm fine with anything regarding what the command should actually be called, I my mind I associated /give with just giving yourself something (as in a cheat command) so I though /gift would be more appropriate. However I can change it to /give if you want? (I could also change it to /give and make /gift an alias or vice versa (just like /g is))?

PhantomGamers commented 8 months ago

How I implemented it now is that the command both works with userId or username (you can use either) if that is not the case at the moment then I made a mistake as it should definitely support both (and I will fix that), does that satisfy your request or is it more about the internals?

I might have just misread the code then, I didn't actually try testing it in-game to see how it functioned, so yeah ignore if that was not relevant my bad.

I am not sure what happens with the whisper command although I expect it just picks the first player matching that username in the list. Now that I think about It my code does the same if you provide a username that is shared by multiple users. I will make it so that it will block the gift in that case and will notify you to use the userId instead with the userIds for that username which are available to your, I think this falls in line with what you suggest?

Yeah I think this would be good, but I mean that doesn't necessarily have to happen in this PR. I think maybe these commands could be restructured so that that the interchangeability between the username and userid would just be available to any command that requires it.

I'm fine with anything regarding what the command should actually be called, I my mind I associated /give with just giving yourself something (as in a cheat command) so I though /gift would be more appropriate. However I can change it to /give if you want? (I could also change it to /give and make /gift an alias or vice versa (just like /g is))?

Yeah the alias idea could work. I understand what you mean about the cheat command though, I was thinking that too, but to me /gift isn't super intuitive as a command for giving items to other players, but yeah having both work could be good perhaps.

One more thing I just thought of but what happens if someone's username is just a number that could be a userid? Seems like this could be a potential problem as well, though an edge case yeah.

R4wizard commented 2 months ago

FWIF when I read the title of this PR, /gift immediately made me think of something like "claim your daily login reward" rather than an admin command to give things.