#Talk about updating plugins by their name instead of file name

1 messages · Page 1 of 1 (latest)

gloomy fern
#

Check the comments, because there is a flaw in your update logic

pliant yew
#

updateNames is a map containing <FileName, File>
The FileName is in no way relevant to their plugin name

gloomy fern
#

Talk about updating plugins by their name instead of file name

gloomy fern
pliant yew
#

Hmm

#

Wait, I think I confused myself there

#

Let me go over it, been a bit since I wrote it

gloomy fern
#

It should ideally go

-> Startup, load all plugin name -> plugin file path

-> Is the update folder a dir?
   -> No? Return the same file
   -> Yes?
      -> Does the same file name exists in the plugins folder?
         -> Yes? Copy it over and return the dest file
         -> No?
            -> Is there a valid plugin loader for the file?
               -> No? Return the file
               -> Yes?
                  -> Does the file have a valid plugin description?
                     -> No? Return the file
                     -> Yes?
                        -> Remove it from the update names
                        -> Copy it over to it's corresponding file path
                        -> Return the updated file
pliant yew
#

I think you're reading everything the wrong way?

#

Let me respond to the comments

gloomy fern
#

Alr

gloomy fern
pliant yew
# 
     -> Yes? Copy it over and return the dest file

You overestimate the stupidity of people, someone will invitably have two plugins with different versions, one matching file name. But complain it didn't update.

#

-> Return the updated file
No. We make sure there's no other files needing an update. Don't return early when we're not in the clear.

gloomy fern
#

@pliant yew Replied to your comments

gloomy fern
gloomy fern
pliant yew
#

No. A version check will never work out.

#

If the user is dumb, we update everything so the user doesn't constantly run into the same issues.

#

They'll run into the wrong version once, and update once.

gloomy fern
#

Alright, so just completely removing old functionality

#

Because removing the file name does nothing, as you're loading it in again right after

pliant yew
#

Have you had enough sleep?

gloomy fern
#

Yes of course

#

I think you're the one lacking on sleep right now

pliant yew
#

If the loader ISN'T null, we don't return here.

What's this supposed to mean?
Isn't this just another way of saying
if loader is null, return

gloomy fern
#

Yeah no I am high as fuck, the first remove is fine because I am apparently a dumbass

pliant yew
#

Yes

gloomy fern
#

yet there is still an issue with how the updateNames are calculated

#

their are looped each time for each plugin update

pliant yew
#

Correct

#

Or better said, they are looped over every time we attempt to load a plugin

gloomy fern
#

Which should only be called on startup/reload, and if a plugin manager wants to update a plugin, why couldn't it use reflection?

gloomy fern
#

Though at least it should be called once per update phase, you know?

pliant yew
#

I agree, but I also believe strongly in "the user is always stupid"

gloomy fern
#

I also agree on that lol

pliant yew
#

Oh right

#

Sec

#

Bukkit.getPluginManager().loadPlugin(dest)

I have this in my plugin actually, its supported behavior

gloomy fern
#

That loads a plugin file directly

pliant yew
#

Yeah

#

It will trigger the update code

gloomy fern
#

What I am talking about is when the server starts it does all of the update -> plugins copying, and we should calculate the updateNames only once there

gloomy fern
#

Well I get why, but why

pliant yew
#

It already triggers the existing update code

gloomy fern
#

Yeah

pliant yew
#

As to why, god knows

gloomy fern
#

Though there should be at least a flag internally for direct update lmao, maybe lets make two PRs, one for this (which would only load updateNames on a reload), and another to add an internal flag to just skip updating

pliant yew
#

I'd rather do the same thing every time, than to change our update process for different scenarios

gloomy fern
#

Because there might be the issue of loading a plugin with the same name as another that's in plugins/ and that would copy it over

pliant yew
#

Can you rephrase that?

#

My only changed thought is that we shouldn't be calling checkUpdateFolder every time we do a global plugin load. Only when we're loading plugins indivially, aka a plugin is calling it

gloomy fern
# pliant yew Can you rephrase that?

Example, a plugin loads another plugin called "SomePlugin.jar"

Source: arbitrary-folder/SomePlugin.jar

if the file plugins/SomePlugins.jar exists the Source file will be copied over as well, overriding the previous file even though that was not the intention

pliant yew
#

Another thought is that we should ignore the cache if we're doing individial plugin loads.
So its always rebuilt on a enableAllPlugins()

gloomy fern
#

We could have a method

PluginManager#updateFile and PluginManager#canUpdateFile for completeness, and PluginManager#loadPlugin (in the API, this has only one arg) will directly load that plugin

pliant yew
#

I'm not following.

Source = The file we're attempting to load
Third Party = plugins/SomePlugins.jar
UpdateJar = plugins/update/SomePlugins.jar

Third Party should be unaffected, since the Source is always carried as a file reference. We never recreate it from the plugin folder

#

Line 461
This is the jar found in update
If this is what you misunderstood

gloomy fern
#

That's not what I meant, let me explain better

Source = SomePlugin.jar placed somewhere in the file system
If a plugin tries to load that jar, and a file with the same name also exists in the plugins folder, it will copy that Source plugin in the plugins folder, even if it was not what the plugin wanted to do

pliant yew
#

Can you rephrase Source plugin, because I don't understand what you mean

gloomy fern
#

Yeah

# 
File we're trying to load: path/to/some/plugin/SomePlugin.jar
Unrelated file inside the plugins/ folder: SomePlugin.jar (Same name, different version/plugin)

If a loaded plugin tries to load path/to/some/plugin/SomePlugin.jar, it will get copied inside plugins/ as a file with the same name already exists, even though that was not what the loaded plugin wanted to do

pliant yew
#

Can you link me to the line where this happens

gloomy fern
#

We try to update a plugin, even if the load was called from a loaded plugin

#

and not from a loadAllPlugins

#

which might not be what the plugin author wants

gloomy fern
#

Whenever loadAllPlugin is called (or whatever the method name is) it will do update checks as well

#

brb

pliant yew
# gloomy fern which might not be what the plugin author wants

This is actually existing behavior, not new behavior.
The existing code already does this, we just reassign the variable name because we now support other file names.

So we have to reassign the variable otherwise we're trying to load a missing file.

So, the real change is that we are renaming your file that you're trying to load. Which may not be what you want.

Eg, you load games/SurvivalGames.jar but it updates, uses update folder name, now is called games/SurvivalGames-beta-build-5476.jar

Which, hmm. I'm not sure what the best solution for that would be, controllable somehow I guess