quisquous / cactbot

FFXIV TypeScript Raiding Overlay
Apache License 2.0
793 stars 378 forks source link

Import remote triggers #2584

Open trim21 opened 3 years ago

trim21 commented 3 years ago

Do you have any thoughts on implementing this feature in cactbot itself?

I didn't know C# well so I'm planning to write a simple wrapper in C# and put most work in another language.

My thoughts are like npm package, describe a remote repo with a json file, and a files filed to describe all content files (without glob support). Other thinga are just like a package manager, when updating, check new version, and replace local file content with new one.

MaikoTan commented 3 years ago
  1. Performance and Safety. When developers upload triggers to cactbot this repo, we have CI to enforce the coding style, we have reviews to ensure code safety, so to ensure safety and performance. But for a remote trigger, we aren't able to check it in runtime. Expecially when you try to add triggers from multiple remotes, users who has not very enough knowledge dont't know what is good and what is bad. I have seen several users who got a bunch of triggernometry triggers, without a zone filter or job filter, then cause ACT stucking. "ACT sucks", they said. Another things that I concerned is safety. Although JS in browser cannot directly access user' files, it can do some HTTP requests, modifying existing code behaviors, and even upload users' private information to somewhere that the user don't want.

  2. Trigger executing confliction. In most content, there are triggers prerequiste others triggers, if we allows multiple remotes, then a couple of conflict would happen. For example, cactbot has a "uwu trigger1"trigger existing. There's a remote A trigger sets overrides the such trigger and set the new property data.garuda = true, then add a new custom trigger "uwu custom A" to read this data.garuda property and do something based on the boolean value. But there are another remote B trigger sets has also a "uwu trigger1" overriding, while it didn't set any new property. Cactbot didn't know anything, it just runs the "uwu trigger1" from remote B, and then run the "uwu custom A" from remote A, and it gives the wrong answer.

  3. Trigger overriding order As described above, when applying multiple overrides, cactbot don't know which should be activated or which should be disabled, so cause the confliction and confusion. "Why can't I add remote triggers from Author Pandragon? There are only a few triggers executed."

quisquous commented 3 years ago

Here are some things that I think would be nice:

1) auto-find cactbot user directory

This often trips users up, so being able to figure out where this is stored would be a huge win.  This may mean that such a plugin needs to be an OverlayPlugin plugin (like cactbot is) so it can use OverlayPlugin APIs to read the config files.

2) sync from git/github

Syncing from a git repo of user triggers would make this easier for other people to use and share (and update).  It may be a bit obnoxious to have a single github repo = a single directory on disk, so it may be nice to have it be possible for one repo to provide several options that people could include/not include?

3) have some way to prepopulate the list of remote repos

Maybe you have some prebuilt json file that lists repos that people can add/remove from as needed, so that they are more easily discoverable?
trim21 commented 3 years ago

auto-find cactbot user directory

There are some problems here (In China) There are two distribution of ACT has most users, one of them has modified OverlayPlugin which didn't support general OverlayPlugin's plugin. (yes, they modified cactbot, too.) An option is to ignore these users(recommended by MaikoTan).

So, if I write an OverlayPlugin, many users in China won't be able to use it.

sync from git/github

I think sync from HTTP is fine. Using git is not an option, as many normal users do not have git installed.

descript a repo with a manifest JSON like this:

{
  "name": "TEA",
  "author": "Trim21",
  "type": "raidboss", // or raidboss only
  "version": "0.0.1",
  "homepage": "https://github.com/Trim21/TEA",
  "files": [
    "./trigger/trigger-1.js",
    "./trigger/trigger-2.js"
  ]
}

files are either an absolute URL or relative path from manifest URL. and the plugin will try to issue an HTTP GET request to the URL.

The authors can still use github as SCM platform, the repo can be downloaded with jsdelivr, rawgit, or github raw.

have some way to prepopulate the list of remote repos

That's definitely the plugin should do.

trim21 commented 3 years ago
  1. Performance and Safety. When developers upload triggers to cactbot this repo, we have CI to enforce the coding style, we have reviews to ensure code safety, so to ensure safety and performance. But for a remote trigger, we aren't able to check it in runtime. Expecially when you try to add triggers from multiple remotes, users who has not very enough knowledge dont't know what is good and what is bad. I have seen several users who got a bunch of triggernometry triggers, without a zone filter or job filter, then cause ACT stucking. "ACT sucks", they said. Another things that I concerned is safety. Although JS in browser cannot directly access user' files, it can do some HTTP requests, modifying existing code behaviors, and even upload users' private information to somewhere that the user don't want.
  2. Trigger executing confliction. In most content, there are triggers prerequiste others triggers, if we allows multiple remotes, then a couple of conflict would happen. For example, cactbot has a "uwu trigger1"trigger existing. There's a remote A trigger sets overrides the such trigger and set the new property data.garuda = true, then add a new custom trigger "uwu custom A" to read this data.garuda property and do something based on the boolean value. But there are another remote B trigger sets has also a "uwu trigger1" overriding, while it didn't set any new property. Cactbot didn't know anything, it just runs the "uwu trigger1" from remote B, and then run the "uwu custom A" from remote A, and it gives the wrong answer.
  3. Trigger overriding order As described above, when applying multiple overrides, cactbot don't know which should be activated or which should be disabled, so cause the confliction and confusion. "Why can't I add remote triggers from Author Pandragon? There are only a few triggers executed."

I'm considering adding an AST parser to parse all js files to extract all trigger IDs and analyze them. But it's not the primary job.

quisquous commented 3 years ago

Yeah, I agree with all of Maiko's points too, but I think that's just fundamental to allowing user triggers and that there's not much you can do about it. I think if you want to add triggers from multiple different authors, then you're into a less-tested and less-compatible zone and there's really no way around that. All of those worries are also true about cactbot triggers, but are just much less likely to happen.

quisquous commented 3 years ago

auto-find cactbot user directory

There are some problems here (In China) There are two distribution of ACT has most users, one of them has modified OverlayPlugin which didn't support general OverlayPlugin's plugin. (yes, they modified cactbot, too.) An option is to ignore these users(recommended by MaikoTan).

If they're going to modify all of that, then arguably they could modify this tool to work with that version of OverlayPlugin/cactbot as well? I agree a little with Maiko here, in that if it's possible to ignore this case, you should go for it.

Mostly I just think "auto-finding cactbot user directory" is a feature that will solve a lot of user confusion.