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

Separate Frontier and NFSD banks, sector-wide bank access, revamp the Station ATM UI #2346

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

Conversation

whatston3
Copy link
Contributor

@whatston3 whatston3 commented Oct 27, 2024

About the PR

Separates out the Frontier and NFSD bank accounts. Extends the BankSystem with function to access sector-wide bank accounts. Most sources of money were moved to only deposit to the Frontier bank account, and bluespace events were changed to pay out the NFSD account.

Also reworks the Station Administration console UI. Adds a separate Withdraw and Deposit sections.

I've left all of the odd taxation sources (vending machines, black market shipyard purchases, black market ATM deposits/withdrawals) going into the Frontier Outpost, this might be a good time to revisit these.

The overall goals of this PR are to give Frontier Staff and NFSD rewards only for relevant activities - good mail performance should not lead to NFSD-wide raises.

Did a bit of cleanup to the MailSystem while I was there, suppressing C# warnings.

NOTE: whatever the outcome of this PR, we need to make sure the NFSD and Frontier accounts can support minimal staffing requirements without shortages. Larger amounts of staff should require competent gameplay to ensure adequate funding for pay.

Why / Balance

With this set of changes, it becomes possible to put a station admin console on the Bottleneck, Wasp, and Empress (didn't do this yet) - the Sheriff and Station Rep gain some autonomy and are less tied to their stations proper. Should help for more off-station gameplay. Payments to Big Ed's Power Plant can be put into a separate account. This should allow better granularity for any additional accounts needed in the future.

How to test

TBD - deposits and withdrawals from NFSD and Frontier consoles should be independent. NFSD and Frontier accounts now have differing ticker rates.

Media

image

Requirements

Breaking changes

Changelog

🆑

  • tweak: The Frontier and NFSD accounts can now be accessed off-station through station admin consoles.
  • tweak: The Station Administration UI has been reworked.

@whatston3 whatston3 marked this pull request as draft October 27, 2024 22:56
@whatston3
Copy link
Contributor Author

Marked as draft, my mistake. Works as-is, but values may need tweaking. A mail metrics-like ledger and end of round stat collection would be nice to have.

@Erol509
Copy link

Erol509 commented Oct 28, 2024

Im affraid of not being able to pay all people, since events are sherrif locked. Yes, you need one to get paid, but joining late and seeing merly enough pay to supply one full shift worker or two is concerning.

Would there be a way to implement a recompensation system, which gives at least 400k cash, in case there is not enough to support people in department?

@dvir001
Copy link
Contributor

dvir001 commented Oct 28, 2024

Im affraid of not being able to pay all people, since events are sherrif locked. Yes, you need one to get paid, but joining late and seeing merly enough pay to supply one full shift worker or two is concerning.

Would there be a way to implement a recompensation system, which gives at least 400k cash, in case there is not enough to support people in department?

We call it Ahelp

@whatston3
Copy link
Contributor Author

Im affraid of not being able to pay all people, since events are sherrif locked. Yes, you need one to get paid, but joining late and seeing merly enough pay to supply one full shift worker or two is concerning.

You have enough, hourly, by default, without events, to cover the minimum wage of the default staff of the NFSD less the sheriff. This is not an issue on the current head of the branch.

@whatston3
Copy link
Contributor Author

Added a ledger at whatston3:2024-10-30-banks-ledger. Modelled largely on the mail metrics app and the logistics stats service backing it, this adds a PDA app (suitable for top-ranking command [currently, SR, Sheriff] use) to view the state of all sector bank accounts at a glance.

Could use a bit of polish, particularly on the blasted PDA app, but if this was hooked into the end of round Discord bot, being able to get an end-of-round summary of all sector bank accounts, their balances, and a breakdown of where their money comes from would be useful.

Interface on current MVP shown below. Top picture shows a shorter list of ledger items, bottom shows the picture with more income line items than will fit on the screen.
image
image

@Cheackraze
Copy link
Member

As dvir said, theres always Ahelp. And, in fact, when this eventually does make it live,, we will likely give our whitelisted players a heads up and remind them that admins are always willing to help provide legit payroll, especially during changes/balance periods.

Copy link
Contributor

github-actions bot commented Nov 1, 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 1, 2024
@github-actions github-actions bot removed the Merge Conflict This PR has conflicts that prevent merging label Nov 2, 2024
@whatston3
Copy link
Contributor Author

Merged the ledger in this branch, current head looks like this.
image

Sector bank and ledger components were merged (avoiding initialization order issues, and combining the interface - all sector deposits now require a reason along with their amount and account, but generate a ledger entry automatically). The LedgerSystem has been included into the BankSystem as a result.

All Bank systems are now namespaced under _NF.

@whatston3 whatston3 marked this pull request as ready for review November 2, 2024 21:09
@Cheackraze
Copy link
Member

who has access to the ledger? I think so long as only SR/Sheriff can view the ledger, this should be good to go

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

github-actions bot commented Nov 7, 2024

RSI Diff Bot; head commit d9d5ad1 merging into b4c69e6
This PR makes changes to 1 or more RSIs. Here is a summary of all changes:

Resources/Textures/_NF/Interface/icons.rsi

State Old New Status
ledger Added

Resources/Textures/_NF/Objects/Devices/cartridge.rsi

State Old New Status
cart-ledger Added

Edit: diff updated after d9d5ad1

@github-actions github-actions bot removed the Merge Conflict This PR has conflicts that prevent merging label Nov 7, 2024
@whatston3 whatston3 added Status: Needs Review This PR is awaiting reviews and removed Status: Awaiting Changes This PR has changes that need to be made before merging labels Nov 7, 2024
@whatston3
Copy link
Contributor Author

Added a printout for the ledger summary at round end. This little test comes out at about 650 characters accounting for formatting, plenty of room to spare for extra info. It's currently using the same webhook URL as the bank printout.
image

Assets added for ledger cartridge and PDA app icon.
image

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
@github-actions github-actions bot removed the Merge Conflict This PR has conflicts that prevent merging label Nov 7, 2024
Copy link
Contributor

github-actions bot commented Nov 8, 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 8, 2024
@github-actions github-actions bot removed the Merge Conflict This PR has conflicts that prevent merging label Nov 8, 2024
@github-actions github-actions bot added the Merge Conflict This PR has conflicts that prevent merging label Nov 10, 2024
Copy link
Contributor

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

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

@dvir001 dvir001 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its working alright, didnt find anything weird.

We will need support for filter some of the SectorBankAccount in the case we add a pirate / syndicate one etc, so you can have a PDA only show the ones relevant to it, this is out of scope for this.

This filter should also support what bank is in what order, so sheriff will have its bank first.

TBH SR Could have kept the WantedList app but its fine.

Please merge this PR unless someone also want to take a look.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C# FTL Map-Outpost Map - Outpost Map-POI Map - POI Sprites Status: Needs Review This PR is awaiting reviews UI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants