Updates OSRS Archipelago Plugin to v1.2

[digiholic]

• Archipelago Client Plugin is now on Maven! No more direct vendoring of plugin code! Nevermind it needed to be vendored anyway!
• Adds new checkbox for toggling AP messages going to OSRS chatbox
• AP Server Password is now an actual password field, using placeholders instead
• Replaces hard-coded tasks with data parsed from github, using version information from Archipelago slot data. This should allow for logic changes that don't require a whole new plugin version to use.
• Adds in a local DataPackage storage, which will store the Slot's connected player and last received item index. This will make reconnection smoother as well as allowing you to see popups for items received while offline.

[digiholic]

Since the automated tests are passing the dependency checks, is there anything else remaining on this before it can get reviewed?

[abextm]

You are writing files outside of .runelite, which is disallowed. You are also using java's built-in object serialization, which we also disallow.

[digiholic]

You are writing files outside of .runelite, which is disallowed. You are also using java's built-in object serialization, which we also disallow.

There are no files being written to outside of .runelite. The generated .save file is in .runelite/APData/.

I will remove the Java object serialization and write my own serializer and parser.

[digiholic]

Serialization has been removed. If I could get clarification on where I'm writing a file outside of .runelite I could fix that

[iProdigy]

You should use @Inject to populate the gson instance instead of doing new GsonBuilder()

[digiholic]

Donezo. Build successful this time, thanks.

[abextm]

You are writing to the cwd in dev.koifysh.archipelago.Client::loadDataPackage which you call (and still uses java serialization). You also are pinning a bunch of deps that I think you don't use, so don't do that.

[digiholic]

That's in an external dependency, would it be sufficient to just override the functions that call those so they aren't used? The pinned dependencies have been found through trial-and-error to be used. The plugin won't build without them.

[abextm]

One of the methods is private, so you can't just override it. I would prefer if they were gone entirely or had to be called manually. Even if you could override it, verifying that you always use the overridden one puts too much burden on reviewers handling your plugin in the future.

Wrt your deps, I deleted half of them and it still built. The error you were seeing earlier is because you didn't have the same excludes in verification-template's build as your plugin's, so it was failing to build because you are pulling more deps there than in your plugin.

[digiholic]

So, at this point I'd need to just embed the source of the plugin instead of using the one on maven, so I could rewrite these methods? Or ask the maintainer of that plugin to add in some way to disable them?

[abextm]

Yes

[digiholic]

Okay, it's been added to the project, and modified to remove all Gsons, Serializables, and writing to improper directories. While I was in there, since the source is now embedded, I was able to excise the dependency on Apache httpcomponents, which means that now the only dependency change is removing one that the original build of the plugin already had.

Hopefully that'll make the process much simpler for review.