search

goatcorp / Dalamud

FFXIV plugin framework and API
GNU Affero General Public License v3.0
1.18k stars 273 forks source link

Feature request: revert "skip plugins that share InternalNames with official plugins" #1020

Open Limiana opened 2 years ago

Limiana commented 2 years ago

Commit https://github.com/goatcorp/Dalamud/commit/a4cac032399914802c42c80833ecb1d1dce7568e This limitation serves no purpose while severely degrading first party plugin development and testing

NotNite commented 2 years ago

How? Do you test plugins on a third party repo? What's the use case?

Limiana commented 2 years ago

Do you test plugins on a third party repo

yes, this is the use case

NotNite commented 2 years ago

In my eyes, you should be using... the testing branch... to test plugins with other users. Rapid development should be done with a local dev plugin.

Limiana commented 2 years ago

Use of testing branch can be quite problematic in certain situations. And it still requires waiting for pr to be approved as well.

NotNite commented 2 years ago

Could you please elaborate on why it's problematic? Also, this does not "serve no purpose" - it prevents people from copying official repository plugins onto repos for things like improperly bumping the API version.

Limiana commented 2 years ago

Well, it can be problematic if you are for example banned from discord server. As for bumping api version, well, you'd need to add third party repo for that anyway which would be kinda step to the unsupported side I suppose? Alternatively can add something like "AllowOverride" flag to manifest, should be not very much hassle?

reiichi001 commented 2 years ago

How would being banned from xivlauncher discord server affect this? (To note, we have only banned bot devs and people who broke our other server rules like spamming, aggressive behavior, inappropriate actions, etc).

We have a number of plugin developers who are not in the discord and who also follow the plugin guidelines fine.

We have, however, had issues with non-developer users making custom plugin repos containing mirrors of official plugins and "revived" official plugins and took actions to prevent that from happening.

If the goal is to rapidly test the plugin with a collection of users, I would recommend changing the internal name to reflect that. Like instead of Plugin, you'd have Plugin [Canary] and users should be directed to disable the stable version and install the unstable version.

PRs of accepted plugins typically are merged multiple times throughout the day, although we now go through a small review process for them instead of just blindly pressing the merge button as soon as possible. And in the case of certain plugin, that can momentarily delay updates.

Limiana commented 2 years ago

The problem is communication channel with users. And, possibly, preference to test it within your own community. I will investigate the idea of having [Canary] versions of plugins, thanks. However, I still would like to ask that override eventually becomes possible one way or another.

NotNite commented 2 years ago

An override defeats the purpose of the check.

Aireil commented 2 years ago

This limitation serves no purpose

The big issue this was solving was the hijacking of main repo plugins from repos where the user did not even download any plugin. However, a later update that fixes the same issue with repos (#992) rendered it kind of useless.

Could probably remove it, although people will still have to manually disable their main repo version and install the repo version as per #992. Then, when you update the main repo, Dalamud will overwrite the repo version and the testers will have to manually install it again next time, could be confusing.

kalilistic commented 2 years ago

In my eyes, you should be using... the testing branch... to test plugins with other users. Rapid development should be done with a local dev plugin.

Unless the policy changed, I thought custom repos are allowed for rapid testing if they only include main-repo-approved plugins. Goat confirmed this recently on 8/26.