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

[EXILED::Loader] Allow port-binded plugin loading #89

Open
wants to merge 15 commits into
base: dev
Choose a base branch
from

Conversation

FoxWorn3365
Copy link

@FoxWorn3365 FoxWorn3365 commented Sep 6, 2024

Description

Describe the changes

With this pr I've made possible to allow EXILED users to load plugins both globally or only binded to a server port.

What is the current behavior?

EXILED loads every plugin found in EXILED/Plugins.

What is the new behavior?

EXILED will load every plugin found in EXILED/Plugins and also the ones in EXILED/Plugins/<server port> where <server port> is the current port.

Does this PR introduce a breaking change?

Nop

Other information:

Fully compatible with the current configuration - an empty folder named as the server port will be created inside the directory
From suggestion: https://discord.com/channels/656673194693885975/663978314683252777/1267269585338372116


Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentations

Submission checklist

  • I have checked the project can be compiled
  • I have tested my changes and it worked as expected

Patches (if there are any changes related to Harmony patches)

  • I have checked no IL patching errors in the console

Other

  • Still requires more testing

EXILED/Exiled.Loader/Loader.cs Outdated Show resolved Hide resolved
EXILED/Exiled.Loader/Loader.cs Outdated Show resolved Hide resolved
FoxWorn3365 and others added 3 commits September 6, 2024 18:40
Suggestion ExMod-Team#1

Co-authored-by: Alex Rouse <123724383+ALEXWARELLC@users.noreply.github.com>
Co-authored-by: Alex Rouse <123724383+ALEXWARELLC@users.noreply.github.com>
private methods should be defined after public ones
Copy link

@iamalexrouse iamalexrouse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything seems ok, just make some adjustments I've mentioned and you should be all good.

EXILED/Exiled.Loader/Loader.cs Outdated Show resolved Hide resolved
EXILED/Exiled.Loader/Loader.cs Outdated Show resolved Hide resolved
EXILED/Exiled.Loader/Loader.cs Outdated Show resolved Hide resolved
FoxWorn3365 and others added 3 commits September 8, 2024 01:03
Co-authored-by: Alex Rouse <123724383+ALEXWARELLC@users.noreply.github.com>
Co-authored-by: Alex Rouse <123724383+ALEXWARELLC@users.noreply.github.com>
Co-authored-by: Alex Rouse <123724383+ALEXWARELLC@users.noreply.github.com>
@FoxWorn3365
Copy link
Author

let's hope to not upset the auto checks

Copy link

@iamalexrouse iamalexrouse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@VALERA771
Copy link

VALERA771 commented Sep 8, 2024

Tbh I don't see point for this pr. We have IsEnabled property in port's config and both current and your system have own dis- and advantages

@FoxWorn3365
Copy link
Author

We don't have to remove the IsEnabled system while adding that system tho.
I used to be a server owner ad you probably know how time-consuming is to enable and disable plugins in every config.
Imagine if you have 3 servers: you'll have to edit 2 <port>-config.yml every time you want to add a plugin only to a specific server.
This system will semplify the job of having different plugins for different servers without going crazy.

@VladTheCow
Copy link
Member

Just use pterodactyl and host servers in different containers 😛

@iamalexrouse
Copy link

Just use pterodactyl and host servers in different containers 😛

This is actually the better approach when working with multiple ports, especially with a game such as SCP: SL Dedicated Server.
Since most server hosts will only be running 1 server, and majority of hosting services will only allow for 1 instance of the dedicated server. So for most "live" use-cases, this feature is kind of redundant.

If you knew what you were doing, you could symlink to a networked location where the plugins are stored, for each port, but in pretty much all use-cases, that's never really been used, or even should be used, for obvious reasons.
For environments like running the server on your computer, this might be helpful, for having specific setups for each port though.

@FoxWorn3365
Copy link
Author

Lots of small SCP:SL servers uses the same machine to host multiple servers.
This feature does not remove anything and only adds something useful, really easy to use and compatible with the previous plugin loading strategy as every plugin that is not inside a folder will be considered global (as it is now).
I don't really see any reason to deny this addition.

@Jesus-QC
Copy link

I love this idea and ill try to make it go forward, but needs more discussion between how we want to structure it.

@louis1706
Copy link

For my pov it's seems ok if you can still show screen shot difference between before and now

@github-actions github-actions bot added Events and removed Installer labels Oct 28, 2024
@FoxWorn3365
Copy link
Author

image
It should looks something like that.
The basic concept is that the main directory (EXILED/Plugins/) will be used for globals plugins while EXILED/Plugins/<port> will be used for port-binded plugins.
For example, if I have two servers inside my VPS (one of the 7777 and the other on the 8888) if i put Scp106Femboy.dll inside the folder 8888 the it will be loaded only on the server on the 8888.
We can also, with a very small addition, just change the global plugin directory from EXILED/Plugins/* to EXILED/Plugins/global/*

@FoxWorn3365
Copy link
Author

We should also add an entry on the log to specify that the plugin has been loaded from a specific folder maybe?

@louis1706
Copy link

We should also add an entry on the log to specify that the plugin has been loaded from a specific folder maybe?

yeah also what would happen if a plugin is in both ?

@FoxWorn3365
Copy link
Author

that's a good good question, I'll have to add a checker.
btw, as I'll have to edit it, should I put global plugins inside a Plugins/global directory?

@louis1706
Copy link

that's a good good question, I'll have to add a checker. btw, as I'll have to edit it, should I put global plugins inside a Plugins/global directory?

actually no idea what the best choice

@louis1706 louis1706 removed the Events label Oct 30, 2024
@FoxWorn3365
Copy link
Author

FoxWorn3365 commented Oct 30, 2024

Maybe a global folder would be more intuitive for the users.
But just to be sure and allow retrocompatibility we can support both methods.
I'll push the update soon

@FoxWorn3365
Copy link
Author

Added the checker and also the global directory

Copy link
Member

@VladTheCow VladTheCow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still disagree with the idea of this PR. It is not needed in my opinion

@FoxWorn3365
Copy link
Author

FoxWorn3365 commented Oct 31, 2024

why?
Pro:

  • would be useful for LOTS of server owners
  • to disable a plugin you won't have to browse a 1100 lines YAML file
  • it's backward-compatible, so they can update without problems and/or whitout having to switch to that system now
  • not everyone uses ptedoractyl

Cons:
nothing?

@louis1706
Copy link

why? Pro:

  • would be useful for LOTS of server owners
  • to disable a plugin you won't have to browse a 1100 lines YAML file
  • it's backward-compatible, so they can update without problems and/or whitout having to switch to that system now
  • not everyone uses ptedoractyl

Cons: nothing?

for me it's fine

@VladTheCow
Copy link
Member

why? Pro:

  • would be useful for LOTS of server owners
  • to disable a plugin you won't have to browse a 1100 lines YAML file
  • it's backward-compatible, so they can update without problems and/or whitout having to switch to that system now
  • not everyone uses ptedoractyl

Cons: nothing?

  • I don't get how it will be useful
  • Just use Split config type and you won't have to browse a bunch of lines
  • It's still not needed
  • They definitely should

@FoxWorn3365
Copy link
Author

Imagine having 100 different files for everyone plugin config and browsing each one only for enabling and disabling the plugin(s).

This addition is backward compatible and will simplify the life of everyone who don't use for a reason or another Pterodactyl.

Something that can help some people is still good to implement instead of saying to them "fuck you and go install pterodactyl just to not become crazy with the configs" or "just have different configuration files for each plugin"

@FoxWorn3365
Copy link
Author

You reasons for that not being needed are:

  • pterodactyl
  • they can enable divided configuration for each plugin

In my opinion these motivations are not sufficient for that kind of opposition

@VladTheCow
Copy link
Member

Imagine having 100 different files for everyone plugin config and browsing each one only for enabling and disabling the plugin(s).

This addition is backward compatible and will simplify the life of everyone who don't use for a reason or another Pterodactyl.

Something that can help some people is still good to implement instead of saying to them "fuck you and go install pterodactyl just to not become crazy with the configs" or "just have different configuration files for each plugin"

It is fine to say "just use the split config type to make it easier"

@FoxWorn3365
Copy link
Author

That was not the point

@louis1706
Copy link

why? Pro:

  • would be useful for LOTS of server owners
  • to disable a plugin you won't have to browse a 1100 lines YAML file
  • it's backward-compatible, so they can update without problems and/or whitout having to switch to that system now
  • not everyone uses ptedoractyl

Cons: nothing?

  • I don't get how it will be useful
  • Just use Split config type and you won't have to browse a bunch of lines
  • It's still not needed
  • They definitely should

it's could be used if you have same plugin with two different version of one plugin that need to be load separatelly also yeah it's just retro compatible so only give more possibility and do not take that much of ressource to load exiled

@github-actions github-actions bot removed the Events label Nov 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants