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

God Wars kill count info boxes #6589

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

Conversation

abd-khrubi
Copy link

This plugin replaces the GWD kc widget with info boxes.

kc-all

@runelite-github-app
Copy link

runelite-github-app bot commented Sep 15, 2024

@DominickCobb-rs
Copy link
Contributor

This would be a great addition to https://runelite.net/plugin-hub/show/better-godwars-overlay

@abd-khrubi
Copy link
Author

This would be a great addition to https://runelite.net/plugin-hub/show/better-godwars-overlay

I respectfully disagree, while both better-godwars-overlay and my plugin are similar I feel that they server different purposes, better-godwars-overlay aims to cleanup the original GWD kc interface while preserving its essence, while my plugin removes the original interface and replaces it with minimalistic info boxes to reduce clutter while doing content in GWD.

I think merging the two plugins together would only hide one functionality behind (somewhat convoluted) option menu, and both plugins can coexist at the same time.

@abd-khrubi
Copy link
Author

But if you still think it's better for them to be the same plugin I can migrate my code to better-godwars-overlay although I see that it's currently hosted not by the original author

@DominickCobb-rs
Copy link
Contributor

You're both modifying the same thing. It would be helpful to have both options in a single plugin so people don't have to guess which it is they want. I'd wait until official word from a RL maintainer says the same thing, but it's a common sentiment with hub plugins that have similar counterparts.

@YvesW
Copy link
Member

YvesW commented Sep 20, 2024

I'd personally recommend contributing to better-godwars-overlay indeed. You could add it as a config option that's disabled by default.
I'm not sure why it's currently hosted as part of Adam's plugins. If Adam just took the plugin to fix an API change, he might even transfer the repo to you if you actually want to maintain the plugin, but that's not up to me.

@abd-khrubi
Copy link
Author

abd-khrubi commented Sep 20, 2024

@YvesW Sure I can add it as a config option to better-godwars-overlay
I wouldn't mind maintaining the the plugin too. @Adam- Are you ok with transferring the plugin to me?

@LlemonDuck
Copy link
Contributor

Yes, we can transfer you the plugin. Adam has it now since it was unmaintained after an update.

Ideally here you would make your changes to the plugin, and then two PRs here on the plugin hub: one changes the ownership to your own repo but does NOT change the commit, for ease of our review. Then also a second PR changing the commit hash to whatever changes you wanted to make.

@abd-khrubi
Copy link
Author

Yes, we can transfer you the plugin. Adam has it now since it was unmaintained after an update.

Ideally here you would make your changes to the plugin, and then two PRs here on the plugin hub: one changes the ownership to your own repo but does NOT change the commit, for ease of our review. Then also a second PR changing the commit hash to whatever changes you wanted to make.

Thanks @LlemonDuck , I'll prepare my changes on top of better-godwars-overlay and in the meanwhile I'll open a PR, like you suggested, to transfer the plugin to me without changes. And then I'll rebase this PR and change it to add my new changes

@LlemonDuck LlemonDuck added the waiting for author waiting for the pr author to make changes or respond to questions label Oct 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
plugin added waiting for author waiting for the pr author to make changes or respond to questions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants