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

Update flipping-copilot to v1.6.0 #7184

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

cbrewitt
Copy link
Contributor

Major refactor and improve transaction inference

  • Refactor all depedencies to use constructor based injection where possible
  • Restructure code to more cleanly follow MVC pattern:
    • Controller/Handler classes should have minimal state
    • State should be stored and managed in 'Manager' classes
    • Manager classes should not reference Controllers or Views (JPanels)
    • Controllers and Views (JPanels) may reference each other
  • Disentangle state from Controller/View classes
  • Update the handling of offer events for inferring transactions based on GrandExchangePlugin code
  • Construct the AccountStatus on the fly directly from client.getGrandExchangeOffers() to avoid sync issues
  • Track the uncollected quantities explciitly in an independent Manager
  • Use accountHash instead of displayName where possible
  • Include a machine ID + additional flags on transactions to help track/clean up bad transactions
  • Simplify the osrs login management
  • Add suggestion settings tab in suggestions panel
  • Move sell only toggle to suggestion settings
  • Add blacklist UI feature

@runelite-github-app
Copy link

runelite-github-app bot commented Dec 28, 2024

@LlemonDuck
Copy link
Contributor

You've done a good job of wrapping things in ClientThread#invokeLater where needed, but you need to be more cognizant of getting back off the client thread as soon as possible. You're doing a whole bunch of I/O and Swing things on the client thread in a lot of places. Use SwingUtilities#invokeLater and a parallel executor instead.

Tip btw: you can use @RequiredArgsConstructor(onConstructor_ = @Inject) if you're just injecting a bunch of private final fields and don't need to do any other actions in the ctor. Then if you extend it you don't have to write it in 3 places.

@LlemonDuck LlemonDuck added the waiting for author waiting for the pr author to make changes or respond to questions label Jan 7, 2025
@runelite-github-app runelite-github-app bot removed the waiting for author waiting for the pr author to make changes or respond to questions label Jan 8, 2025
@cbrewitt
Copy link
Contributor Author

cbrewitt commented Jan 8, 2025

Thank you for the feedback, we've made the following changes to address it:

  • Ensure all ui refresh methods execute in swing thread
  • Switch disk writes in most Manager classes to async
  • Use @requiredargsconstructor where possible

We've also added a few other changes with the latest commits:

  • add f2p only toggle
  • fix "collect items" bug

@LlemonDuck LlemonDuck added the waiting for author waiting for the pr author to make changes or respond to questions label Jan 10, 2025
@cbrewitt
Copy link
Contributor Author

what do you need this for? cbrewitt/[email protected]:74b567cf386eeb28709c814afa5026ad76225f3f#diff-143066f71dc6a3c22ce21eaf87df1118bdbed9c53de9e4d22568a65829f77952R223-R263

This is actually borrowed from in the GrandExchangePlugin code https://github.com/runelite/runelite/blob/0c0c41d5b7d6b1314d2257ffb9c10cf82b2ec7b9/runelite-client/src/main/java/net/runelite/client/plugins/grandexchange/GrandExchangePlugin.java#L938-L981. We thought that it could be useful in certain cases for ensuring transaction consistency and debugging user reported issues related to transactions.

Right now our backend isn't actually doing anything with it. If there are any concerns relating to user privacy/security we can remove it.

@runelite-github-app runelite-github-app bot removed the waiting for author waiting for the pr author to make changes or respond to questions label Jan 10, 2025
@LlemonDuck
Copy link
Contributor

If you aren't using it now I would prefer if you could remove it, yes.

@LlemonDuck LlemonDuck added the waiting for author waiting for the pr author to make changes or respond to questions label Jan 11, 2025
@runelite-github-app runelite-github-app bot removed the waiting for author waiting for the pr author to make changes or respond to questions label Jan 12, 2025
@cbrewitt
Copy link
Contributor Author

MachineID has been removed now, as requested

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

Successfully merging this pull request may close these issues.

2 participants