Closed DanielNoord closed 3 years ago
It's the return string of the function that is being matched. This is still needed.
Edit: Notice that Beautify(Game.Objects[name].getPrice())
is not in quotes. You won't find it in the source CC. It's a dynamic string.
Edit 2: Added more more comments in #355
Yep I understand that Beautify(Game.Objects[name].getPrice())
is a dynamic string, but it looks like the computed string is never matched in l('tooltip').innerHTML
.
I'll add an example to illustrate this:
In my current test save, the original l('tooltip').innerHTML
value when trying to sell 1 cursor is:
<div style="min-width:350px;padding:8px;">
<div class="icon" style="float:left;margin-left:-8px;margin-top:-8px;background-position:0px 0px;"></div>
<div style="float:right;text-align:right;">
<span class="price">2.419 Quint</span>
<div style="font-size:80%;opacity:0.7;line-height:90%;">Infinity worth<br>0% of bank<br></div>
</div>
<div class="name">Cursor</div>
<small>[owned : 300</small>] <small>[free : 10</small>!]
<div class="line"></div>
<div class="description">Autoclicks once every 10 seconds.</div>
<div class="line"></div>
<div class="data">• each cursor produces <b>0</b> cookies per second<br>• 300 cursors producing <b>0</b> cookies per second (<b>0%</b> of total CpS)<br>• <b>68.82 Quindec</b> cookies clicked so far</div>
</div>
and the value of Beautify(Game.Objects[name].getPrice())
, matched for the split is, 5.3 Quint
, which corresponds to the effective price of buying 1 cursor. However, this string 5.3 Quint
is nowhere to be found in the original string, leading to the split/join having no effect and the tooltip HTML being left unaltered.
I can reproduce this both in single and bulk selling.
What I understand is that the intent of the code is to match the price in <span class="price">2.419 Quint</span>
in order to replace it with a computed price (starting by a minus to show this is a sale).
Maybe the game was wrongfully displaying the "buy" price instead of the "sell" one when being in sell mode, and this snippet of code was fixing it with the real sale price; however this is not doing anything anymore as the game shows the right "sell" price now.
For reference, the piece of code we're talking about is:
else if (Game.buyMode == -1) {
if (Game.buyBulk == -1) {
l('tooltip').innerHTML = l('tooltip').innerHTML.split(Beautify(Game.Objects[name].getPrice())).join('-' + Beautify(CM.Sim.BuildingSell(Game.Objects[name], Game.Objects[name].basePrice, Game.Objects[name].amount, Game.Objects[name].free, Game.Objects[name].amount)));
}
else {
l('tooltip').innerHTML = l('tooltip').innerHTML.split(Beautify(Game.Objects[name].getPrice())).join('-' + Beautify(CM.Sim.BuildingSell(Game.Objects[name], Game.Objects[name].basePrice, Game.Objects[name].amount, Game.Objects[name].free, Game.buyBulk)));
}
}
And my current test save:
I see what you mean. Indeed in a newer version of CC this may be the case. In this case, I will agree this is legacy code, though maybe not as I would still like to add the minus sign. CM may now be looking for the wrong value. If i get a chance, I will look more closely, but I am unsure when that would be. If you could be so kind to look in my place @DanielNoord.
Edit: I took a look and indeed the number has changed. I guess Orteil did fix it, lol. I still would like to add the minus however, so I think the code just needs to be reworked to add the minus now.
I agree with @Chorizorro that this seems to be legacy code. The game now calls Game.Objects[object].bulkPrice to determine which price to display. And this does display the correct price.
CM is thus not even calling the correct price. I don't really understand why we would want to add the minus to the tooltip. I understand that you get sort of a 'minus price' when selling something, but I think it is actually quite clear already. Adding the minus might confuse people.
I've been doing it a bit more testing, and it appears that the Game.Objects[object].bulkPrice
computed by the game is still incorrect, whereas the price computed by CM.Sim.BuildingSell
appears to be right.
This piece of code is therefore still relevant and needs to be updated. (I've a fix that is almost ready, I just need to find the time to test it and push it)
Are you sure? I just took a quick look at the source code and the function this.buy
for buildings calls this.getPrice()
.
.bulkPrice
calls .getSumPrice
which seems to be the same code, just with a different amount.
Therefore, I don't see how Game.Objects[object].bulkPrice
can return an incorrect sell price.
However, you have put much more time in this section of the code so please tell me if I overlooked something!
While playing without any add-on loaded, I noticed that the amount of cookies I was earning by selling a building was different to what the game showed me beforehand, and I could reproduce it multiple times. However I don't know what causes the Game.Objects[object].bulkPrice
to be incorrect. I think I'll investigate a bit further into this to make a bug report.
Okay! Let me know if I can help anywhere!
I was finally able to understand what was going on after a few hours playing with the debugger.
There is only one tiny difference between the code calculating the selling price of a building, and the code processing the sale. Considering n the current number of the object we want to sell, and amount the amount of object we want to sell:
Game.Object.getReverseSumPrice
lines 6917 to 6927 (click to expand)Game.Object.sell
lines 6974 to 6994 (click to expand)From a fresh new game, no upgrades, no ascension, no auras... The simplest possible case. I currently have 7 cursors. The last bought cursor (7) cost me 35 🍪, the next one (8) will cost me 40 🍪.
The Cursor frame and the tooltip shows me that, selling my last bought cursor (7) will give me 9 🍪 which seems to be correct:
Cursor.basePrice
is 15, the one we're selling is the 7th, the Game.priceIncrease
is a constant 1.15, Game.modifyBuildingPrice
doesn't change the price and the sell multiplier is the default 0.25. Giving us:
⌈ 15 1.15^(7 - 1) 0.25 ⌉ = ⌈ 8.67 ⌉ = 9 (The reason why we have (7 - 1) as price increase exponent is because there is no price increase for the 1st boughe object, meaning it starts from 0 for the object 1)
However, the selling method uses the current price of the object, which corresponds to the price of the NEXT object to be bought, resulting in earning 10 🍪 by selling the last bought cursor (7).
The getPrice
method returns this:
⌈ 15 * 1.15^7 ⌉ = ⌈ 39.9 ⌉ = 40
which is then computed into the following:
⌊ 40 * 0.25 ⌋ = ⌊ 10 ⌋ = 10
I think this means that selling buildings always gives more 🍪 than it should, because the base price used is the one of the next object to be bought, not the price of the last one bought.
Bulk selling suffers the exact same problem: when selling 10 objects, the tooltip will show an amount based on the sum of the 10 last bought objects, whereas the sell method will compute an amount based on the price of the next object + the 9 last bought objects.
There is always an offset of 1 object between the base price calculated in the display, and the one calculated during the sale.
Even if the effective sale price of objects seems to be the one invalid, Cookie Monster needs to display the real number of 🍪 earned by selling a building.
The CM.Sim.buildingSell
should be able to do that easily.
There is currently a bug making the 🍪 earned by selling a building always be calculated with optimal auras (ES + RB), but I've already found a workaround.
Very good work! You might want to submit a bug-report to Orteil, as I think he need to choose which price he wants to actually use. In the meantime we can make CM display the real number. I do think it will be good to annotate that extensively, maybe referring to this issue, so when Orteil fixes this bug we can immediately change CM.
With regards to the second bug, I assume you will tackle both when you submit a PR that fixes this issue?
Thanks! Yes I'm going to do that. Do you know if there is an easy way to submit a bug report to Cookie Clicker by any chance? The only way I've found so far is by joining their Discord, which annoys me a bit.
Absolutely, the upcoming PR should fix everything! Hopefully I will be able to push this today.
Not that I know of. Perhaps via Twitter or tumblr? I don't see any other communication channels on the site and his personal site...
Thanks for the confirmation.
Yup, I think I'll take the time to go on Discord and talk a bit about this issue anyway, just to make sure it isn't already a well-known and documented issue and not bother Orteil with it if it's not necessary
With #372 I think we can keep this issue closed right?
Yes, I think so, as the bug is now fixed! (I may post one more comment in this issue if I have any feedback from Orteil / the Discord community)
Quick update: I just reported it on the Cookie Clicker Discord, it seems like the bug was unknown and has been added to a "to-report" list. Also, some people were happy to know that they weren't going nuts by always earning more 🍪 than expected when selling their buildings 😄
I've noticed that this isn't actually doing anything, as
Beautify(Game.Objects[name].getPrice())
is never matched in the string; maybe this is some legacy code to remove?_Originally posted by @Chorizorro in https://github.com/Aktanusa/CookieMonster/pull/355#discussion_r531712864_