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

New admin command: spawnrefund #2359

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

arimah
Copy link
Contributor

@arimah arimah commented Nov 2, 2024

About the PR

  • Adds a new admin command: spawnrefund
  • Adds a new admin log type: AdminRefund

Why / Balance

Every once in a while, admins have to issue refunds. The server might crash, or a player sells someone else's ship, or a bug causes someone to be deleted, or raiders kill everyone on Frontier. Whatever the reason, refunds are sometimes needed. Unfortunately spawning exact amounts of spesos is awful. You have to spawn stacks of spesos and combine or split, which takes ages. Most admins seem to just round up to the nearest reasonable multiple so it doesn't take a decade. It's also a bit of a nightmare to ensure you don't drop money on the ground, as you can't spawn things directly into your own hand.

The spawnrefund command is intended to help with all of this.

Usage: spawnrefund <amount> [reason]
Examples:

  • spawnrefund 52000
  • spawnrefund 52000 "for JeanGreytide, other player sold their ship"

To limit abuse, the command imposes the following restrictions:

  • You must be an admin with ADMIN privileges. TODO: Verify that this is the correct privilege.
  • You must be a ghost. Stops potential badmins from spawning money whenever they need it. All our admins are of course beautiful people, but Frontier has downstreams, too. :)
  • You must have a free hand. The command spawns the money directly into your hand.

The command logs every successful refund to the admin log. For ease of tracking, the message includes the word "refund", and there's a whole new log category called AdminRefund.

The command is called spawnrefund rather than something more generic, like spawnspesos or spawnmoney, because admins shouldn't really be spawning money for anything other than refunds.

How to test

  1. deadmin and try to use the command. Be rejected (and dejected).
  2. readmin and possess a body. Use the command, be told you have to be a ghost.
  3. ghost and use the command again. Be told you need a free hand.
  4. aghost and use the command again. Get money in your hand.
  5. Use the command again. Get money in your other hand. Verify no money drops on the ground.
  6. Use the command again. Be told you need a free hand.
  7. Try to spawnrefund hello. Be told hello is not a valid integer.
  8. Try to spawnrefund 0 and spawnrefund -100. Be told the values are out of range.

Media

Success

image

Admin log:
image

Admin log message includes:

  • The words "refund" and "spesos" for ease of finding without having to filter by log type.
  • A reference to the spawned entity ID, so it can be tracked trivially, if necessary, without having to find a separate log message.

Failure states

Non-admin attempt:
image

Admin, not attached to a ghost:
image

Non-admin ghost:
image

No free hand:
image

Also shows a brief popup over the ghost in case you type the command into the chat console (/spawnrefund ...):
image

Argument validation errors:
image

Requirements

Breaking changes

None.

Changelog
🆑

  • add: There is a new admin command, spawnrefund, for spawning an exact number of spesos. To be used for the rare refund only, as the name suggests.

@arimah
Copy link
Contributor Author

arimah commented Nov 2, 2024

This PR is labelled "Need Discussion" so that it can be discussed among admins, to make sure I haven't missed anything crucial.

@whatston3
Copy link
Contributor

whatston3 commented Nov 3, 2024

A few points and questions.

  1. While the use case you've given makes sense, would it be more helpful to have a panel to modify a player's bank account (both deposit and withdraw)? Something like the station ATM interface (deposit/withdraw, amount, reason), and accessible through the player info panel similar to Whitelists Panel (DeltaV) #2152? Not sure how easy these tools are to use, but a GUI and searchable window might make sense.

    1A. Motivating use case - Joe Dirt logs on, steals 25k spesos from Stan Upstanding on Frontier - you can make Stan whole easily, but are Joe's ill-gotten gains safe?

  2. Spawn privilege would make sense, no? You're spawning an entity.

  3. While we don't have a changelog at the moment for Frontier-specific admin tooling, we could, and this would make sense to go there (along with Whitelists Panel (DeltaV) #2152 :^) ).

@arimah
Copy link
Contributor Author

arimah commented Nov 7, 2024

@whatston3

  1. Though there may be value in being able to edit people's bank accounts directly, e.g. if people log off or there a lot of reimbursements to process, spawning cash is something admins are already quite familiar with. It also gives players instant confirmation that a refund has indeed been issued. While I could implement something like that, I think a simple command is an okay first step. :)

    As for your motivating example 1A, if someone is found to have taken another's money, we want to force them to go to an ATM, withdraw the money, and hand it over. It's a crystal clear show to the person that their ill-gotten gains are indeed not safe, it means we get to guarantee a conversation with them, it means we can ban them if they fail to comply, and more.

  2. That DOES make sense, but focuses more on the mechanics of the command. I think ADMIN is probably fine because refunds are an administrative action. :)

  3. We could! If we get an admin changelog, I'd happily change my PR. Until then, however... :D

Copy link
Contributor

github-actions bot commented Nov 7, 2024

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

@github-actions github-actions bot added the Merge Conflict This PR has conflicts that prevent merging label Nov 7, 2024
@whatston3
Copy link
Contributor

whatston3 commented Nov 7, 2024

  1. We could! If we get an admin changelog, I'd happily change my PR. Until then, however... :D

https://github.com/new-frontiers-14/frontier-station-14/compare/master...whatston3:2024-11-07-multiple-changelogs?expand=1

The Delta-V changelog job that we've based ours off of doesn't use a few of the newer changelog features (admin-only flags, order, etc.), I've done a little bit of editing of the changelog.js script so it shouldn't clobber any of these changes we've made to the file locally, and can refer to a few known changelog files from the contents of the PR.

Largely untested, might pick it up tomorrow. (why is it written in node, nothing else is written in node)

@arimah
Copy link
Contributor Author

arimah commented Nov 7, 2024

Conflicts fixed, history slightly cleaned up. This is ready to go if there are no changes you want me to make. :D

@github-actions github-actions bot removed the Merge Conflict This PR has conflicts that prevent merging label Nov 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants