Process items in player inventory fix (For 914)

[Undid-Iridium]

Versus original

(tl;dr process player didn't process items in player inventory)

[IRacle1]

It's ok, your fix is working, but one thing unrelated to your PR I didn't like

// setting = ev.KnobSetting
new(OpCodes.Callvirt, PropertyGetter(typeof(UpgradingInventoryItemEventArgs), nameof(UpgradingInventoryItemEventArgs.KnobSetting))),
new(OpCodes.Starg_S, 4),

KnobSetting setter after UpgradingInventoryItem event, I dont like thats it sets argument, because you can set KnobSetting for first item, and it will affect not only the first item

[joker-119]

Process player wasn't really meant to process their inventory items, that's handled by UpgradingInventoryItem event. Just sayin, this isn't a 'fix', this is a redesign.

[Undid-Iridium]

Process player wasn't really meant to process their inventory items, that's handled by UpgradingInventoryItem event. Just sayin, this isn't a 'fix', this is a redesign.

That does not seem correct. I didn't rewrite the code, I simply fixed what it was supposed to originally do. If you see ProcessItem (I think, don't quote me), it does the same kind of logic. Furthermore, all I did was make sure it could actually iterate the items it was supposed to (because it was not), I didn't add any logic beyond that. Though it's been 20 days since I last looked at this so - possibly incorrect; however, from my quick glance, pretty sure my understanding is correct. I'll double check later but I am pretty sure it is a bug. Thanks

[Undid-Iridium]

It's ok, your fix is working, but one thing unrelated to your PR I didn't like

// setting = ev.KnobSetting
new(OpCodes.Callvirt, PropertyGetter(typeof(UpgradingInventoryItemEventArgs), nameof(UpgradingInventoryItemEventArgs.KnobSetting))),
new(OpCodes.Starg_S, 4),

KnobSetting setter after UpgradingInventoryItem event, I dont like thats it sets argument, because you can set KnobSetting for first item, and it will affect not only the first item

I'll take a look later

[joker-119]

I think we're miscommunicating the 'bug' here a bit.

It wasn't the intention of the UpgradingPlayer event to also handle the actual upgrading of the items in their inventory, however it was the intention to be able to deny upgrading of inventory items within this event.

The items upgrading seperately is intended. The items upgrading regardless of the UpgradingPlayer.UpgradeItems property, is a bug.

[Undid-Iridium]

Firstly, for the first picture, if you do not remove the check given by NW, whatever their answer is for whether this should be allowed based on 914Inventory will be there - I removed it as I would imagine a user might want to ignore what NW designed.

Secondly, with the original code, it was offset 13 from ldsfld, which in this case, put it in a random spot/incorrect spot because this is a for loop of all the players items they are currently holding.

Thirdly, I cannot imagine that being correct, especially for what it was trying to do, which was, process each item in the players inventory. Previous continue would not have achieved that (see picture of BR_S which it was not pointing to).

Consequently, this means that this ProcessPlayer class is supposed to process each item. If "It wasn't the intention of the UpgradingPlayer event to also handle the actual upgrading of the items in their inventory" is supposed to be true, then the second half of this transpiler should not exist.

Let me know if I missed anything, thanks

[Undid-Iridium]

Closing due to inactivity from reviewers. If someone wants this, re-open or create new PR.