plugin-hub icon indicating copy to clipboard operation
plugin-hub copied to clipboard

Create recommended-equipment

Open t8or opened this issue 1 year ago • 25 comments

Added Recommended Equipment plugin. This awesome plugin takes your OSRS gameplay to the next level by seamlessly integrating equipment recommendations from the wiki, via the approved wiki scraper, directly into your RuneLite client and then filtering your bank, similar to Quest Helper.

It helps you gear up quickly for bossing and mini-games by using wiki data without ever leaving RuneLite.

t8or avatar May 15 '24 05:05 t8or

New plugin recommended-equipment: https://github.com/t8or/runelite-recommended-equipment/tree/da919e4bb4394c4d2de186561017a979a00ff541

image I'm not sure why it's saying it needs to wrap at 120 when it's at 114 🤔

adamk33n3r avatar May 15 '24 05:05 adamk33n3r

https://github.com/runelite/plugin-hub/blob/d40f8d47274a84dbe0a33a5b4db8d218db5082be/package/package/src/main/java/net/runelite/pluginhub/packager/Plugin.java#L400

raiyni avatar May 15 '24 05:05 raiyni

oh fun.....lol why EIGHT for tabs :lul:

adamk33n3r avatar May 15 '24 05:05 adamk33n3r

that's the default size of tabs in github's ui

Nightfirecat avatar May 15 '24 05:05 Nightfirecat

alright, i converted the file to spaces so hopefully that works. but i dont have permissions for @t8or's fork at the moment to update the hash, but he can do it in the morning. https://github.com/t8or/runelite-recommended-equipment/commit/a41ed77fe623873159332087e76af9bdf2af49eb

adamk33n3r avatar May 15 '24 05:05 adamk33n3r

You should absolutely not be mutating event objects, that would negatively impact every other plugin which would be listening for it. What's going on with RecEquipLAF? Why does it require reflection? How is that needed here? I'm generally a little confused about all this LAF code (both for this file, and the use of RuneLiteLAF) you have going on. You add your nav button here which is never removed, even on plugin shutdown. Please ensure it is removed when disabling your plugin.

Nightfirecat avatar May 24 '24 01:05 Nightfirecat

Hey nfc, I'm the dev on this project.

You should absolutely not be mutating event objects, that would negatively impact every other plugin which would be listening for it.

The code in the banktab package is more or less copied from QuestHelper. That line specifically is also in quest helper here: https://github.com/Zoinkwiz/quest-helper/blob/master/src/main/java/com/questhelper/bank/banktab/QuestBankTab.java#L227 I don't know exactly what it's doing, or if it's needed for our case. I just tried to keep everything there.

What's going on with RecEquipLAF? Why does it require reflection? How is that needed here? I'm generally a little confused about all this LAF code (both for this file, and the use of RuneLiteLAF) you have going on.

The reflection specifically is copy/pasted from the RuneLiteLAF to get the colors from the colorscheme into the style file. As for what RecEquipLAF is for, it's to customize the style for our own panel. Whenever our panel is open, we switch to RecEquipLAF and when our panel is closed we switch back to RuneLiteLAF. You can see that here. Unfortunately, swing doesn't really support multiple LAF's so this was the cleanest solution I could come up with. You can see our stylesheet here

You add your nav button here which is never removed, even on plugin shutdown. Please ensure it is removed when disabling your plugin.

Yep, that's totally my bad. Speaking of shutdown, should we be removing the sprite override as well? (if possible)

adamk33n3r avatar May 24 '24 01:05 adamk33n3r

  1. You should be doing what bank tag layouts does

  2. You should be able to set whatever properties on the object. Swapping in a new LAF is not good.

raiyni avatar May 24 '24 01:05 raiyni

Hey Rayini,

Can you explain why we should be doing what bank tags does instead of quests helper?

Can you point us in the direction to set visual design properties on the objects instead of using a LAF? We couldn't find an easy way to adjust objects that repeat many many times; hence creating the LAF.

t8or avatar May 24 '24 02:05 t8or

More specifically, rounded buttons. Can easily make a rounded border, but the background would still be a rect. flatlaf has the capability to do full rounded buttons. Maybe some manual paintComponent override or something? 🤮

I think you can set a client prop on a button to get the rounded button visual, it was just way easier to combine all styles into a stylesheet. I can look into what it would take to do everything in code.

I do wonder why swapping in a new LAF is not good though. Isn't that how apps switch themes?

adamk33n3r avatar May 24 '24 02:05 adamk33n3r

the laf is global, you are not allowed to set it

abextm avatar May 24 '24 02:05 abextm

and really, you should not be using rounded buttons because nothing else in the client uses rounded buttons; your plugin integrates into the client, not the other way around.

abextm avatar May 24 '24 02:05 abextm

This is how the plug-in should look. Would love direction on how to edit each object so it inherits this styling.

image

t8or avatar May 24 '24 02:05 t8or

Swing doesn't support multiple styles in a single application, you should inherit the styles provided by the LAF so it looks like the rest of the application.

abextm avatar May 24 '24 02:05 abextm

Appreciate the direction Abex!

t8or avatar May 24 '24 02:05 t8or

I'm not a developer/maintainer of this particular repo/application, nor do I have any say in the approval of this PR. I'm just a software engineer who follows chatter on repos and PRs I find interesting (like RL and this plugin in particular).

Another way to word this is that you want your plugin's panel to feel like it's part of the core RL app (even though we all know it isn't). And if you stray too far from the core with your own LAF it can feel jarring to the end user and feel like it's own app inside of RL (instead of a core app/plugin, like mentioned before).

That is all, back to lurking. Good luck with your plugin, I'm looking forward to seeing it on the hub!

Intecpsp avatar May 24 '24 10:05 Intecpsp

Alrighty, removed the custom LAF and removed that event mutation 👍🏻

adamk33n3r avatar May 25 '24 02:05 adamk33n3r

It's been a little while. Just wanted to check in and see if there is anything else to fix before merging into the plug-in hub. Thank you!

t8or avatar Jun 16 '24 06:06 t8or

Core is adding bank tag layouts and will be exposing an API to register your own layouts, recommend looking at that.

raiyni avatar Jul 02 '24 16:07 raiyni

@raiyni do you know when that API will be available?

I'm anxious to get this plugin released as I know it will be a great QoL update for all the new equipment and combat changes that just came out.

t8or avatar Jul 02 '24 18:07 t8or

It's already on master, so next release. https://github.com/runelite/runelite/commit/ee0c998621dde080c80a968bdf35e773783271d1

YvesW avatar Jul 02 '24 20:07 YvesW

Just so I can make sure I understand, this PR won't be merged until the plugin is changed to use the API that will be out soon?

t8or avatar Jul 03 '24 05:07 t8or

Does this new API let us create sections and dividers like we have in this plugin? image

I looked through the commit you linked @YvesW but I'm not seeing anything that would let us do that, the sections especially are important.

adamk33n3r avatar Jul 05 '24 01:07 adamk33n3r

That is not currently supported.

geheur avatar Jul 05 '24 01:07 geheur

With the announcement of Bank Tags in the new official client, is that using the same API as the one above? I hope that this plugin can be integrated before the great plugin migration.

t8or avatar Jul 25 '24 02:07 t8or