Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix on shop related issues and the remove-x on resizable issue #68

Merged
merged 2 commits into from
Jan 17, 2021

Conversation

lmsv-mx123
Copy link
Member

@lmsv-mx123 lmsv-mx123 commented Jan 16, 2021

Fixes #64, fixes #67, and fixes #50. Issues related to shop and (although unrelated) the remove-x in trade screen issue

@lmsv-mx123 lmsv-mx123 requested a review from Hubcapp January 16, 2021 18:15
@Hubcapp
Copy link
Member

Hubcapp commented Jan 17, 2021

#50 behaviour before:
Crashes when remove X from trade
#50 behaviour after:
Dialogue comes up fine & is usable to remove X from trade. 👍

#64 behaviour before:
Array Index Out of Bounds crash when shop index reference is set to -1
#64 behaviour after:
Client detects that shop index = -1, and aborts attempting to send the packet, preventing client crash 👍
#64 possible improvements (for later PR if desired):
Could store reference to item ID selected when dialogue box came up, then search for new shop index. This way the user would not have their input discarded.

#67 behaviour before:
Attempting to buy/sell more than Short.MAX_VALUE (65535) overflows the 2 bytes, value sent to server would be (Value % 65535).
#67 behaviour after:
Input is discarded if it is not in bounds between 0 and 65535. Not actually sure this is a strict improvement, because at least some of what the user wanted to sell/buy was executed, but now none of it is.
#67 possible improvements:
Input should not be discarded, but instead should truncate to 0xFFFF if value > 0xFFFF

@lmsv-mx123
Copy link
Member Author

lmsv-mx123 commented Jan 17, 2021

#67 possible improvements:
Input should not be discarded, but instead should truncate to 0xFFFF if value > 0xFFFF

Placed, agreed possibly best choice. Was a bit undeciding to unbound or cancel if it was like due to some typing error, like trying to sell 10k but instead mistyped to 100k

#64 possible improvements (for later PR if desired):
Could store reference to item ID selected when dialogue box came up, then search for new shop index. This way the user would not have their input discarded.

This has to be made for some future PR possibly even with config option. Not sure if @Hubcapp wants to keep it under same issue or new one, since technically now would fall under enhancement

@Hubcapp
Copy link
Member

Hubcapp commented Jan 17, 2021

Placed, agreed possibly best choice. Was a bit undeciding to unbound or cancel if it was like due to some typing error, like trying to sell 10k but instead mistyped to 100k

Ahh, yeah. that makes sense. In OSRS for Sell-X, they just bound it high up, don't reject. But I can see that reasoning.

@Hubcapp
Copy link
Member

Hubcapp commented Jan 17, 2021

Anyway, I'm going to merge, and all 3 issues are resolved. I will place a new issue for #64 possible improvement, which may or may not ever get done and it would be OK.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants