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 and rename nex-split-tracker to osrs-splits-the-kodai #7211

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

Conversation

KeyboardIsMagic
Copy link
Contributor

Integrated the features of osrs-splits proposed plugin to the already existing plugin. Changed name to reflect merge.

Integrated the features of osrs-splits proposed plugin to the already existing plugin. Changed name to reflect merge.
@runelite-github-app
Copy link

runelite-github-app bot commented Jan 6, 2025

@tylerwgrass
Copy link
Member

Instead of deleting the old plugin file and creating a new one you can instead just replace the repo and hash in plugins/nex-split-tracker

@KeyboardIsMagic
Copy link
Contributor Author

I didn't think I deleted the old plugin file, I did overwrite that same repo with the new commit which did replace a lot of files and restructured the repo. I made a new fork, changed the name, and updated the url/hash. I'm sorry for any complications this made.

@YvesW
Copy link
Member

YvesW commented Jan 6, 2025

So generally you want to...

  1. Not rename the pluginfile in this repo
  2. Add the new config options to your config (and keep the old one), can potentially add sections.
    This way it's very easy to combine your old and new code. Functions can just be disabled via the config.
  3. Keep the old configgroup (or write migration code that runs in startup and profilechanged IIRC but there's no reason to do that when the old group works).
  4. You can change the name of Plugin.java but IIRC you need to define the old name so it remembers it's on/off state. You can check at what I did with the TimersPlugin when I renamed that (Team plugin has the same IIRC).
  5. If you add new functionality that does http requests (not to github/runelite.net/osrs.game/osrs wiki) and the plugin did not have a warning before or the warning does not encompass all new http functionality, you want to put the http functionality behind a config option that's disabled by default and has a warning attached to it.

@YvesW YvesW added the waiting for author waiting for the pr author to make changes or respond to questions label Jan 6, 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 6, 2025
@KeyboardIsMagic
Copy link
Contributor Author

KeyboardIsMagic commented Jan 6, 2025

-- changed the name osrs-splits-the-kodai back to original -> nex-split-tracker.

-- changed the config group back to original "nexsplittracker"

-- added ConfigItem enableExternalSharing with an informational warning and wrapped all Http requests around this check.

PluginDescriptor name remains "OSRS Splits - The Kodai" as I believe that is a better name for the plugin, I have not yet looked into YvesW suggestion "You can check at what I did with the TimersPlugin when I renamed that (Team plugin has the same IIRC)." Which I will do now and either update the commit or come back here with questions. Thanks for the patience.

@YvesW YvesW added the waiting for author waiting for the pr author to make changes or respond to questions label Jan 6, 2025
@KeyboardIsMagic
Copy link
Contributor Author

KeyboardIsMagic commented Jan 6, 2025

Could you link me the suggested plugin for reference? I look at your repos, and the plugin-hub for "time" and couldn't find the reference you mentioned. Or if I can get away with directly renaming PluginDescriptor that would be tight.

@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 6, 2025
@Nightfirecat
Copy link
Member

He's talking about the change he made to the core Timers & Buffs plugin (formerly it was the Timers plugin): runelite/runelite@06de8a6

@YvesW
Copy link
Member

YvesW commented Jan 6, 2025

I meant the commit NFC linked above indeed! Mostly aiming at configName = "TimersPlugin",

@YvesW YvesW added the waiting for author waiting for the pr author to make changes or respond to questions label Jan 6, 2025
@KeyboardIsMagic
Copy link
Contributor Author

I understand, thank you.

@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 6, 2025
@KeyboardIsMagic
Copy link
Contributor Author

KeyboardIsMagic commented Jan 12, 2025

I've made those changes, Is there anything else I need to do on my end?
Not sure if I am waiting for review or something else.

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