bahrmichael / factorio-tycoon

GNU General Public License v3.0
8 stars 6 forks source link

feat: introduce city debt + fix free construction #312

Closed winex closed 1 week ago

winex commented 3 months ago

fixes

tl;dr

naming was choosen in perspective of paying-to-player and to look similar, not something like increaseDebt() and addCurrencyToTreasury()

as i promised - with this you'll able to repay UPC leftovers with just payCurrency() to any and every city in #294 what do you think?

todo

winex commented 2 months ago

rebased on latest main, removed goto statements, resolved cost to be nil on requirements failure

looks much better now, marking as ready just to request changes...

bahrmichael commented 2 months ago

Those are really great changes and I'll be happy to merge them in after a quick test.

Just had one comment about informing players what "unpaid debt" means for them, but we can also do that post-merge.

winex commented 2 months ago

oh, editing from github was too weird for me. do we actually need both tooltips?

anyway, i must rebase all of these because of conflicts, so i'll add fractional part into second tooltip instead, but integer debt/"pending payout" will be shown

edit: wow, this works awesome, it's just "Pending payouts:" doesn't have rounded i marker, but adding [virtual-signal=signal-info] just after caption looks very ugly. so, we'll just live without it

bahrmichael commented 2 months ago

do we actually need both tooltips?

I would suggest adding it for the label and the value, so that it's easier to display the hover and learn more.

bahrmichael commented 2 months ago

Code looks overall good to me, let me know when I should merge it :)

winex commented 2 months ago

testing this together with #304, looks cool for the quick tests

i'll test (and probably stream) this more tomorrow.... still need to reduce logging a bit, but we need that especially when testing multiple cities/multiple treasuries

edit: oh, no construction cost constant for tycoon-bottle-return-station and it's not listed in special building. i'll provide a fix below...

winex commented 2 months ago

i waited so long for used-bottle-store be merged just to make this commit: 79b286a , haha

funny, doing tests on my first save, i've found a bug: #321 ...

bahrmichael commented 2 months ago

i waited so long for used-bottle-store be merged just to make this commit: https://github.com/bahrmichael/factorio-tycoon/commit/79b286acafa74d7b918325db1f37b4a65cf809c9 , haha

Glad that's done now! :D

bahrmichael commented 1 week ago

I copied and merge the branch. Thank you very much for this great upgrade!