BottomlessBag

1 - Popup Dialog always says "profit" - misc other errors

I love this addon.  I use it constantly.
I have one serious complaint, though.... sometimes, it lies!

There are various places in the code where a construct similar to:
        DROP_DEL_3..BottomlessBag_MoneyFormat(Lootprice-Lowestitem)
is used in the middle of a BBAG_MSG() call to display the profit obtained by dropping the Lowestitem item in favor of the candidate (Lootprice) item.  Unfortunately _MoneyFormat always displays the absolute value of the price it receives, so if the value happens to be negative, there is no indication of this.  In many places where this type of code exists, there is no check made to test if Lootprice < Lowestitem, and there is only one message (i.e. DROP_DEL_3) that is ever displayed; and that single message explicitly says "... profit of ".  Thus a net LOSS is displayed as a profit, and as the absolute value no less so there isn't even a minus-sign in the output to clue you in.

Some attempt was made to fix this for specific parts of the code, and a "... loss of " message (i.e. DROP_DEL_3b) added to be displayed if an explicit check was made and found that the candidate item was lower-valued.  But this type of check is not made everywhere; where not made, you get the "profit" message even for a net loss.  One condition in particular where it is NOT made is when
  - you have PlayerCleanCorpse set, and
  - you have ScanDropAndLootEnabled unset, and
  - you loot an item when inv is full

This pops up the "BAGS ARE FULL! Trying to delete <item> (value) A potential profit of ..." dialog, to select between keeping or deleting the item.  localization.lua provides a DROP_BAGS_FULL_4 ("...profit of") and a DROP_BAGS_FULL_4b ("...loss of") message, but the code never even references 4b; the message is always "profit" regardless of profit or loss.

This is obviously completely wrong in many cases.

I've patched up my copy to do a few cleanup things.  I've torn out all the "profit" and "loss" text from the localization strings, created a new pair of localization strings PROFIT_OF and LOSS_OF, and added a function BottomlessBag_ProfitOrLoss() that tests the sign of the value passed and displays  PROFIT_OF..BottomlessBag_MoneyFormat(value) or LOSS_OF..BottomlessBag_MoneyValue(value) as appropriate, and then used this new function in all the places where the original code used the _MoneyValue() function to display profit or loss valuation.  This means the value is always tested everywhere, eliminates extraneous localization strings that differed only in profit vs loss (and for which German versions didn't even exist), and always prints the proper text to describe reality.
I also cleaned up a number of typos ("dropable" -> "droppable", "Skiping" -> "Skipping", etc).
Finally, I also added a contains() function, and replaced some of the code like
                elseif strfind(name,"Oozing Bag") or strfind(name,"Slimy Bag") or strfind(name,"Scum Covered Bag") or strfind(name," Clam") or strfind(name," Muschel") or strfind(name,"Schleimbeutel") or strfind(name,"Schaumbedeckter Beutel") or strfind(name,"Schlammiger BeutelSludge-covered Object") or strfind(name," Clam")then

with code like
       elseif contains(OPENABLES,name) then
and made OPENABLES another localization entry, for ease of maintenance and internationalization.

I would be delighted to provide the changes to you, in the hopes you will adopt them and make this addon even better!

--Rubio

User When Change
bugmenot2 Fri, 07 Nov 2008 06:22:16 Create

You must login to post a comment. Don't have an account? Register to get one!

  • 1 comment
  • 1 comment

Facts

Reported on
07 Nov 2008
Status
New - Issue has not had initial review yet.
Type
Defect - A shortcoming, fault, or imperfection
Priority
Medium - Normal priority.

Reported by

Possible assignees