Bug - Fixed Divide by zero error eating quantum taco/ consuming Astral Energy Drink

I don't think we have any issues because of ambiguity.

Any program that does division without checking for a zero denominator or doing its best to guarantee a non-zero denominator is flawed at least in Ye Olden Days when numerical analysis was my bread and butter.

Fixing that makes the problem go away. The PR has not been "released" because the concoction data is different when the tests are run in isolation and in the test suite. I am claiming that is because TCRS tests leak the state of the consumables database but I cannot yet prove it to my satisfaction. I am working on that.
 
Are we sure? The error with div/0 was happening with the astral energy drink in the real world experience in the start of this thread, despite the two AED’s each having a value for SpleenHit.

I don’t really know why we look up the item by name when usesRemaning requires an itemId anyway
 
I am sure. Look at the test for https://github.com/kolmafia/kolmafia/pull/1759.

If there is benefit to also testing the pure string "astral energy drink" then we can add it but how does that string get to the code?

I understand the following to mean the ambiguity has been dealt with prior to the computation.

Ryo_Sangnoir said:
Why is "astral energy drink" getting passed anyway? If we know what you want to consume, we know what the ID is, so we should be able to pass the correct item.
Click to expand...

As I said on the PR there are two issues.

One is the question - is the test adequate for the divide by zero cases. If someone thinks that is not the case then new tests need to be written and added.

The other is that TCRS tests leak and I had to disable them to get the divide test to work.
 
Added
Code: 
assertThat(ConsumablesDatabase.getRawSpleenHit("[5140]astral energy drink"), is(8));
assertThat(ConsumablesDatabase.getRawSpleenHit("[10883]astral energy drink"), is(5));
assertThat(ConsumablesDatabase.getRawSpleenHit("astral energy drink"), is(0));

I'm also not seeing anyplace where the non-test code calls ConsumablesDatabase.getRawSpleenHit with anything other than a concoction name so I feel like there is a lot of concern about something that never happens outside of an artificially created test.

Maybe we are in different places because as far as I am concerned I have already fixed the divide problem?
 
I haven’t reviewed the code (still at work). Does this solve Adventuristo’s original problem with a scripted use of the Astral Energy Drink?
 
MCroft said:
I haven’t reviewed the code (still at work). Does this solve Adventuristo’s original problem with a scripted use of the Astral Energy Drink?
Click to expand...
I believe so unless my belief that "ash chew" leads to a UseItemRequest is incorrect.

I started with a failing test that failed because of a divide by zero and while there have been some distractions along the way the test now passes :-)
 
Thanks for the fix!

My first clue that there was something wrong was today when my daily script tried to drink 196 Schrödinger's thermoses. It had been acquiring but failing to drink for that long. Near the end of the script, so missed by my inattention.
 
fractalnavel said:
Thanks for the fix!

My first clue that there was something wrong was today when my daily script tried to drink 196 Schrödinger's thermoses. It had been acquiring but failing to drink for that long. Near the end of the script, so missed by my inattention.
Click to expand...

Just for my piece of mind you are reporting that things work now and had been broken for perhaps 196 days?

Fixing this was a slog, some of that self-inflicted, and I don't wish to revisit it immediately :-)
 
You must log in or register to reply here.
Back
Top