aclist / kbin-kes

Add-on manager and scripting framework for kbin
MIT License
24 stars 8 forks source link

Support other kbin* TLDs #13

Closed aclist closed 1 year ago

artillect commented 1 year ago

I wonder, do the mods even need the @match block to even work (and the rest of the meta block as well)? Since they're just being included as raw JS files I don't think that'd have any impact, I think we only need to add the extra domains to megamod.user.js

aclist commented 1 year ago

Don't think so, provided it's all in the top script. We'd need to audit what includes the mods are calling and whether anything is not making it through. I was also thinking it might make sense at some point to merge all of the functions into one function definition file and load that--consider the pros and cons here. The /mods directory would contain the atomic archive of the mods-as-scripts, while the functions file would roll up those mods into a single script of functions.

I also externalized the CSS in the main branch into a file that gets loaded. So you would have:

I don't know if GM can load an external includes file, but probably not.

Something else to think about if we reach a point where a maximalist config page is needed: https://stackoverflow.com/questions/14594346/create-a-config-or-options-page-for-a-greasemonkey-script

Oricul commented 1 year ago

From what I've seen with kbin-code-highlighting being implemented here is that the entire userscript header is ignored for child userscripts. Almost no point to them for child scripts other than notation.

aclist commented 1 year ago

Makes sense, and since child scripts are not updating themselves, that information is superfluous. We might look into consolidating these as best we can into a collection of functions, as alluded to above. My rationale is that I want to limit the GET requests made to GitHub when loading the remote resources for the first time. It doesn't make sense to make 100 requests for 100 atomic scripts and get rate limited. They should be cached after that point, but it seems unnecessary when we could make a single request and be done. The /mods directory could persist as an archive of scripts in their atomic state.

aclist commented 1 year ago

N.B. There seem to be quite a lot of kbin instances, refer to @Oricul's script or look for the canonical list of instances

Oricul commented 1 year ago

This is where I got my list from, and I'm sure it's out of date already.

artillect commented 1 year ago

That matches up pretty well with the list here, I think it's pretty exhaustive

aclist commented 1 year ago

Hmm, 60+ instances in the includes is a bit extreme from a maintainability perspective. One alternative would be to give the script a global match scope, load the instances list from a file into an array, and test if the indexOf of the current window location exists in the array, then proceed.

The downside is that this feels intuitively wrong insofar as it's triggering on every possible page you'd visit, but it's not functionally different from what GM is itself doing when testing the match scope.

I can't come up with a security negative to this at the moment, because nothing else is being executed and presumably the file is cached.

While the match logic in the includes supports globs, the instance names could have any arbitrary name.

artillect commented 1 year ago

I'm gonna go ahead and add the biggest kbin instances to the list, just to cover as many people as possible. In case someone's instance isn't supported right out the gate, worst case scenario they just have to manually add it. I think this'll work well enough for now since the vast majority of people are on kbin.social and fedia.io anyways

Edit: It seems like fedia.io and karab.in were added in @Oricul's PR, ignore what I said lol, maybe this issue should be closed? I'm not sure if you wanted to go forward with the custom matching logic @aclist

aclist commented 1 year ago

Oricul's PR added kBin labs; I added fedia.io and karab.in a few commits ago since they were in the top three or four.

I don't know about matching all of them, could just leave this as tech-debt for now and reopen if necessary.