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

Ectoplasmator reminder #7326

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

Conversation

staytheknight
Copy link
Contributor

Changed the structure of the plugin to fetch a list of NPC names from another branch, and compare the name of the NPC targets (Instead of IDs).

This is to set up a structure for future updates, where only the text file on the other branch needs to be updated with new Spectral NPCs, and no changes need to be made to the function or structure of the code on main.

Updated commit version
Was using @ConfigGroup("example") - cause an issue, @ConfigGroup has been renamed
Added Varlamore Part 2 Creatures. 
Frost Nagua & Amoxilatl boss
Changed the structure of the plugin to fetch a list of NPC names from another branch, and compare the name of the NPC targets (Instead of IDs). 

This is to set up a structure for future updates, where only the text file on the other branch needs to be updated with new Spectral NPCs, and no changes need to be made to the function or structure of the code on main.
@runelite-github-app
Copy link

runelite-github-app bot commented Jan 31, 2025

@LlemonDuck
Copy link
Contributor

You're not closing this reader: staytheknight/ectoplasmator-reminder@7b8c959...staytheknight:830e603cfc5589c707a37a6db21d5cd1a3073e12#diff-10693da25e57a0233faf318338fc9fb18252e411e8edc6a852d65756c8fdc750R53-R54

Also you should not block the EDT while it downloads these files on plugin start up. You can inject the OkHttpClient and use that which has support for async operations built-in.

@LlemonDuck LlemonDuck added the waiting for author waiting for the pr author to make changes or respond to questions label Feb 1, 2025
@staytheknight
Copy link
Contributor Author

You're not closing this reader: staytheknight/[email protected]:830e603cfc5589c707a37a6db21d5cd1a3073e12#diff-10693da25e57a0233faf318338fc9fb18252e411e8edc6a852d65756c8fdc750R53-R54

Also you should not block the EDT while it downloads these files on plugin start up. You can inject the OkHttpClient and use that which has support for async operations built-in.

Great, thank you for the feedback, I will look into fixing these issues, and trying to async this process.
I appreciate the heads up as to what I should look into as I'm not very familiar with I/O.

@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 Feb 1, 2025
@LlemonDuck LlemonDuck added the waiting for author waiting for the pr author to make changes or respond to questions label Feb 1, 2025
Changed URL Reader to Async OkHTTP calls
@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 Feb 1, 2025
@iProdigy
Copy link
Member

iProdigy commented Feb 1, 2025

Please use the version of okhttp already provided by the runelite client rather than depending on 4.12.0 (simply inject OkHttpClient rather than try to construct it)

@iProdigy iProdigy added the waiting for author waiting for the pr author to make changes or respond to questions label Feb 1, 2025
@staytheknight
Copy link
Contributor Author

staytheknight commented Feb 2, 2025

Please use the version of okhttp already provided by the runelite client rather than depending on 4.12.0 (simply inject OkHttpClient rather than try to construct it)

Thanks for the heads up, first time using OkHTTP for me, had a lot to figure out :)
Changing it to @ Inject now.

@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 Feb 2, 2025
Changed OkHTTP calls from using a gradle / maven import, to an @Inject to call built in plugins.

First time using OkHTTP so I did not know what I was doing.
@staytheknight
Copy link
Contributor Author

Looks like it didn't like me calling 'new OkHttpClient()'.
Trying to fix that now, I'm just confused how to properly use @ Inject.

@tylerwgrass tylerwgrass added the waiting for author waiting for the pr author to make changes or respond to questions label Feb 2, 2025
@staytheknight
Copy link
Contributor Author

https://github.com/runelite/runelite/blob/13073fd235b386f17640bdef53468d23194446c8/runelite-client/src/main/java/net/runelite/client/plugins/crowdsourcing/CrowdsourcingManager.java#L50-L51

just inject and then you can use it, you do not need to call new

Thank you very much for the examples, examples help me tremendously.
I had my new code set up exactly as yours was with the Inject.
My Inject was not working as it kept giving me a null pointer.

Looking more closely at the differences between our code, I found out the class storing my OkHTTPClient inject was 'abstract' and my IDE kept telling me to add static in front of a lot of my function calls.

Removing Abstract and Static modifiers and changing how the SpectralCreature URL reader is called in the Plugin seems to have fixed my issues on my side of things after testing, we'll now see if the automatic checks on GitHub like it or not.

Java is not a language I use often if at all, so this has been quite the adventure.

@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 Feb 2, 2025
Removed abstract class modifier from SpectralCreatures class so the @ Inject for OkHTTPClient could work. 

While the class was abstract, my IDE kept telling me to make everything static. 

I had to go change how SpectralCreatures is called in the plugin using an @ Inject. 

This seems to have fixed all the issues on my side for testing, now we will see if the automatic GitHub checks pass
@LlemonDuck LlemonDuck added the waiting for author waiting for the pr author to make changes or respond to questions label Feb 2, 2025
Changed console prints to logger
@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 Feb 2, 2025
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.

4 participants