-
Notifications
You must be signed in to change notification settings - Fork 94
feat(display): Implement player money per minute #1481
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
base: main
Are you sure you want to change the base?
Conversation
{ | ||
incomeStr.format(L"%d", cashPerMin); | ||
} | ||
buffer.format(TheGameText->FETCH_OR_SUBSTITUTE_FORMAT("GUI:ControlBarMoneyDisplayIncome", L"%ls (%ls)", moneyStr.str(), incomeStr.str())); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about the Money symbol?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The limitation here would be the ControlBarPro money texture. It has much less space compared to the original game's ControlBar, hence I removed it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you show image for it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You tested Control Bar Pro correctly in 16:9.
The Original Control Bar always needs to be tested in 4:3. That is because its elements are stretched wide in 16:9 and beyond.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
void Money::updateIncomeBucket() | ||
{ | ||
UnsignedInt frame = TheGameLogic->getFrame(); | ||
UnsignedInt lastSec = m_lastBucketFrame / LOGICFRAMES_PER_SECOND; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this value not be derived from m_currentBucket
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think so, m_lastBucketFrame
records the exact frame of the last update
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But do we care for this frame number? We only care for bucket position, no? I would expect that this logic can be simplified.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
UnsignedInt lastSec = curSec - ((curSec + ARRAY_SIZE(m_incomeBuckets) - m_currentBucket) % ARRAY_SIZE(m_incomeBuckets));
What pain points does this feature solve? Is it not something skilled players can already intuit? Are the benefits worth the added complexity / cost to readability? |
Only game I know of that inlines the resource per min data point similar to this is Supreme Commander. But that game is a supply chain simulator compared to ZH. Starcraft 2 has an income tab in replay mode only, so you can't see it while you're playing. income/min is useful if you're studying your replays and helps spectators understand the economy game, but I reckon not so much for the player himself. |
A lot of people who watch Legis stream are often interested in his |
I think this is a nice addition. I'd append the unit "/min" though to make it clearer what it means. |
I don't think this is possible while remaining ControlBarPro addon compatibility as It has much less space in the money texture compared to the original game's ControlBar |
Even if it overflows, it's fine. Someone will make an updated ControlBarPro addon soon that fixes the texture. And until then, you can still disable it. |
{ | ||
for (UnsignedInt i = 0; i < 60; ++i) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about we just call init() here if that sets everything to 0 already?
m_startingCash = 0; | ||
m_currentBucket = 0; | ||
m_lastBucketFrame = 0; | ||
for (UnsignedInt i = 0; i < 60; ++i) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be simplified/optimized with std::fill
m_money = amount; | ||
m_currentBucket = 0; | ||
m_lastBucketFrame = 0; | ||
for (UnsignedInt i = 0; i < ARRAY_SIZE(m_incomeBuckets); ++i) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be simplified/optimized with std::fill
diff = 60; | ||
m_currentBucket = (m_currentBucket + diff) % ARRAY_SIZE(m_incomeBuckets); | ||
for (UnsignedInt i = 0; i < diff; ++i) | ||
m_incomeBuckets[m_currentBucket] = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks wrong now.
Could probably also use std::fill here.
@@ -105,6 +120,10 @@ class Money : public Snapshot | |||
|
|||
UnsignedInt m_money; ///< amount of money | |||
Int m_playerIndex; ///< what is my player index? | |||
UnsignedInt m_startingCash; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this for? It appears to do nothing.
for (UnsignedInt i = 0; i < 60; ++i) | ||
{ | ||
m_incomeBuckets[i] = 0; | ||
} | ||
} | ||
|
||
void init() | ||
{ | ||
m_money = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like init()
is now synonymous to setStartingCash(0)
, so that can be simplified.
@@ -462,6 +462,7 @@ void Player::init(const PlayerTemplate* pt) | |||
{ | |||
m_money.deposit( TheGlobalData->m_defaultStartingCash.countMoney(), FALSE ); | |||
} | |||
m_money.setStartingCash(m_money.countMoney()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs another look. First depositing money and then setting starting cash is not good.
It should be one call.
Better design setStartingCash
in a way that prior calls to deposit and similar are not necessary.
UnicodeString moneyStr; | ||
UnicodeString incomeStr; | ||
|
||
if (currentMoney >= 100000) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code portion can become prettier by breaking out the money values formatting:
UnicodeString moneyStr = formatMoneyValue(currentMoney);
UnicodeString incomeStr = formatIncomeValue(cashPerMin);
I'd leave the money display alone, and put this data into top-left corner together with other GenTool things. I'd be interested in seeing this data, but putting it next to current money might hinder comprehension speed |
Perhaps it would be better suited for an Observer specific display. |
Frankly, it would be great fun to see this while playing singleplayer |
The money display is left alone. This feature is disabled by default now. I think it makes more sense this way. |
This change implements player money per minute in Zero Hour.
Tracks earnings over the last 60 seconds. By default, it is enabled by "yes" and can be disabled by the value "no" in Options.ini.
By disabling it, it will fall back to original current money only.
For example with Mauller #1442
TODO