feat: add TRSBankItem

[Torwent]

This commit adds a new record called TRSBankItem.

TRSBankItem = record
    Item: TRSItem;
    Quantity: Int32;
    Noted: Boolean;

    Tab: Int32;
    Scroll: Int32;

    IsSetup: Boolean;
  end;

It's purpose long term would be to completely replace TRSBankWithdrawItem and TRSBankDepositItem. Due to how widely used this are I choose to keep them for now. Marking methods that use them as deprecated could be studied.

I also made some changes to the core TRSBank by adding the currently global constants of the file directly to it as class constants. The idea would be for this to also replace the current constants in the future completely but I also left them for the same reason I left TRSBankWithdrawItem and TRSBankDepositItem. It's widely used.

TRSBank.WithdrawHelper() was edited to cache the last withdrawn custom quantity. This is something I've been doing for a long time in WaspLib and has been working great in my opinion. Reading up text while the bank is open has a decent probability to fail, I don't know the odds but it's higher than normal, this mitigates the issue by a lot, by keeping the last withdrawn value cached in a new class var added to TRSBank called CachedQuantity.

Lastly, a new TRSBank.WithdrawItem() and TRSBank.DepositItem() that use the new TRSBankItems were added.

[Torwent]

Also to be clear, don't accept this yet, needs more testing and probably some tweaks, I just want some feedback on whether this is a good direction or what

[slackydev]

Yeah. I think it's acceptable as a change. Not a fan of breaking changes, but keeping the old methods as deprecated makes it passable.

As we talked about: The bank uptext issue when in fixed mode (possibly also when in resized but tiny window) can be fixed with the OCR. But for now it's not possible due to a bug in SimpleOCR, we need to upgrade for latest OCR and test to see if it's stable for usage at some point, and at which point the fix can be added.

For now cache is acceptable. But should be a temporary solution.

---

The TRSBank datatype code may need some tweaking, as is, it's a mess. Also would prefer the type to defined higher up.. before any functions.

---

Not a fan of:
procedure TRSBankItem.Setup(item: TRSItem; quantity: Int32 = -1; noted: Boolean = False);

To setup something as simple as a bank-item.. I'd rather it would just be a simple method BankItem(x,y,z). IsSetup concept as well, I dont think that is needed. I dont want to safeguard everything, and I'd want us to be able to reuse variables. Meaning if we went that way with a setup-function, I would want users to be able to use the same instance of a variable for several setups and withdrawals/deposits.
But I'd much rather see a simple method.

---

I'd also prefer that TPA, ATPA and acronyms like this continue to be capitalized like in our natural language. It's just a preference.

PS: left some comments in the code regarding this, and one small mistake.

[Torwent]

Hi @slackydev ,

The first point, if OCR can indeed be fixed for this and not fail I agree and I don't mind testing it once it gets updated.

Regarding the TRSBank type, I completely agree, it should be one of the first things people see in the file. I think it could be moved up without any issues.

As for the TRSBankItem.Setup() I'm not sure I understand what you mean. You mean just removing the IsSetup variable?
Like so:

type
TRSBankItem = record
    Item: TRSItem;
    Quantity: Int32;
    Noted: Boolean;

    Tab: Int32;
    Scroll: Int32;
  end;

procedure TRSBankItem.Setup(item: TRSItem; quantity: Int32 = -1; noted: Boolean = False);
begin
  Self.Item := item;
  Self.Quantity := quantity;
  Self.Noted := noted;

  Self.Tab := -1;
  Self.Scroll := -1;
end;

That can be done. Tbh, .IsSetup doesn't even make much sense, it's just that I was originally thinking of something else, I was thinking to have the Bank.WithdrawItem() and Bank.DepositItem() checking the IsSetup variable and calling TRSBankItem.Setup() if it was false but I ended up scratching the idea since the user already has to set the item name, quantity, noted, etc.

This change can be made, IsSetup is not doing anything useful.

Anyway, I'm still playing with this and this is not ready yet, I'm actively tweaking things for better functionality, I'll commit more changes when I have them nailed down. I also, made some changes to scrolling through wasplib overrides which I think should definitly go into SRL eventually but I think that should go on a different PR another time.

IIRC, SRL uses the scroll bar by clicking the the exact range you choose. That is.... so bot like. But I guess it could be argued, I just don't like it I guess.
With wasplib I have it using the actual bankscreen and using the mouse scroll wheel which is how I think it should be.
Anyways, the result is technically the same and as I said, that should go on a different PR another time in my opinion.

[Torwent]

It might also not be a bad idea for this to be a function that returns a TRSBankItem That way it could be used on the fly if required.

[slackydev]

Yeah, sounds like you got the message the way I intended it. And yea, I prefer PRs separated, to not mix in scroll changes here.

[Torwent]

After having tested this for a week, I think it's in a good state already. It has some quirks if you are using a script that has items in 2 different tabs at 2 different zooms but honestly, That's a very bad user setup to begin with and it will still work, just won't be optimal.

Aside from the changes you suggested like moving the bank record up, what I changed is that the cached scrolls will get updated every time the item is visible and we are trying to withdraw it, line 1178.
I've made this change because when withdrawing multiple items overtime the scrolls will even out to something that works for all items. This is however, only if the items can all be visible in a single bankscreen, if you really need to scroll every time to every item, it's gonna be quirky but I would seriously consider that a terrible setup by the user anyway.

With that said, I'm going to release this to SRL-T today and will be start adding it to my scripts and see how it goes if you want to wait for some real usage information!

[slackydev]

Alright this looks acceptable. Thank you.