-
-
Notifications
You must be signed in to change notification settings - Fork 21
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
bug: manifest.json
does not uniquely identify a Thunderstore mod
#165
Comments
Potential solutions would include putting the author name into Thoughts? |
I don't love the idea of editing extracted files post-install, I'd rather add a new file with the mod string, perhaps called something like |
Yeah, not editing any existing files causes the least issues. Though I'd change the filename so it's not hidden (on *nix) by default, so no leading dot and maybe something that is rather verbose, so that any random player checking their mod files can guess what it does, so like for example Maybe also with The other question is, do we wanna put the whole modstring in there, i.e. |
files downloaded from thunderstore are called packages, so |
I think the general idea should be, to not attempt to populate I think, also as stated above, a simple plaintext file with the author name, is all that's needed. And then it's up to the individual launcher/manager to use it, when distinguishing packages. Something as simple as In reality, this is sort of just an issue created because of Thunderstore not deciding to include the author inside the manifest file (although I get why that's not the case), and Northstar not requiring an author value either. But I digress. |
I still think the best way about this is to have the mod.json change. Since we have direct and easy control to that. In the case of the author's really needing iden, wouldn't adding the author's name to the mod.json once you unpack the mod. @GeckoEidechse Northstar does not break with extra Val's in the JSON right?. |
Mandatory author name's in the mod.json would be the fastest imo. If it comes down to it, A mod_manifest?, Probably dump it next to the mod.json. |
While I like the idea of the author being strictly in the On top of this, it'd be user editable, unlike with getting the Thunderstore author. So any mod author could put anything in there, not ideal. We have basically all the data we'd ever need in the Therefore, I still think a simple plaintext file like |
That would not help anyway, cause we need Thunderstore author string here, not Northstar mod author (they can differ) :P
Northstar handles it just fine ye, been doing it for a while without issue but I wanna switch away cause just like @AnActualEmerald, I don't love the idea of editing extracted files post-install.
I'm kinda tending towards If we do go for "just author", I'd say the file should be named |
I feel 1 file which solves all of this would be ideal. That way future issues are resolved by this as well, it's best Imo that we establish a long-term solution. |
I did forget to mention here, that I don't actually think it would solve the problem, as it would allow anybody to edit the file and add a name that isn't their own.
It really does not require more work, In my opinion
While I get the sentiment with having a file with more information than just author name, and making it possible to easily extend it. It could cause issues, it's not as likely, but interoperability especially between different versions of various launchers/managers could be hard to ensure. But in case we do go with a slightly more extensible format, we could simply have a file like {
"author": ""
} The major issue here, is if we start adding many things in here it could overcomplicate the file, more importantly, if in the future something is changed, like a key is renamed or similar, we would potentially break backwards compatibility and interoperability. Compared to having a simple And so the problem here is really just interoperability and backwards compatibility, if each launcher/manager used it's own file and format, they, if responsible, would work in backwards compatibility of some kind. However if we have a shared way of doing things, and we make a change that requires updated code to support backwards compatibility and or not cause problems, we'll inevitably have someone who isn't compliant or isn't so instantly. But that's just my view on it, at least... |
In fact I think if we would decide to one day go from
That being said, for FlightCore I just use @AnActualEmerald's library and don't really implement much myself, so I'm fine with leaving the final decision of whether to do single |
Not entirely. In the case of a |
JSON would be nice for parsing reasons. |
I would argue it'd be far easier, simply read the file, and maybe remove leading and trailing whitespace, and or only read the first line. Compared to parsing JSON and hoping it's not formatted horribly for whatever reason. |
But JSON formatting is easy, and you just have to read for the var you want?. |
Whatever the format is, I'm good with it, just wanted to say, if we are looking at shared data now. |
I was thinking that if 1 manager wanted to become the main handler, should we check for other installs of other managers?.(if viper wanted to handle links but r2mm existed, r2mm would be default. Etc..) |
That's what you have libraries for :P
What specifically do you have in mind? The |
Personally I'm not planning the to add one cause the current Thunderstore mod integration in FlightCore turned out to be more/as usable as Thunderstore website itself. The way I know URI handlers, if there are multiple registered, you'll be asked to pick one each time. At least that's my experience with URIs in Firefox. So it might just be fine to register anyway and the user will get the option or it will override it, idk. I'd suggest continuing URI handling discussion in #124 as ebkr who develops r2mm also commented there about it. |
Given you've dealt with the Thunderstore and mod installing, I would assume you've also dealt with how terribly people can mess up something as simple as JSON. The problem is not really that JSON parsing is hard, the problem is that going from basic plaintext to a format like JSON adds far more potential for edge cases, and possible errors that can occur.
We've actually had an issue for that before (#124), and my opinion from it remains the same as at the time, and just to sum it up here: I don't think URI handlers are bad, however we'd need a new handler at not something like Changing the handler however has backwards compatibility problems... Furthermore, I don't see the biggest reasoning behind having such a handler, especially with Viper's mod browser effectively being equivalent to browsing the Thunderstore web page, unless it's because you've been sent a link and you want to quickly open it. But I am not directly against adding it, if done properly. |
I found that it tends to just be JSON comments or trailing commas that are the issues. Given that those are all in spec for JSON5 I just switched my JSON parser with one that supports JSON5 and so far didn't have any issues since ^^ (Between URI handling and this, we're starting to stray off-topic from the original here btw :P) |
While JSON5 is still a great idea, and also was when I first heard about it, it's major problem is backwards compatibility, and in our case as well, interoperability, not staying to the widely used spec/parsers means other launchers/managers will now either update to JSON5 as well, or repair the JSON themselves, once again, more possibilities for edge cases and errors. So it's far more ideal for everybody, to make it try and parse with a wider supported spec, and repair if able, and in any other case throw an error, as to not cause any other problems.
Mostly agree here, on that off-off-topic perhaps the R2NorthstarTools org should have a sample repo just to contain mod manager discussions or just discussions overall. |
In the end what matters is whether Northstar supports it, cause that's what's needed for Northstar to load the mod and also what most mod developers will use for "testing". So I don't see any need to "repair" JSON files which are supported by Northstar anyway. I feel it's rather the responsibility of the mod-manager author to ensure they are still able to parse the JSON file for their own needs. Again using JSON5 libs seem to deal with it just fine and if it they don't, TL;DR: Assume that what Northstar supports is the "standard" and just adhere to that instead of trying to ensure everything is vanilla JSON. |
I'd be in favor of a plain Perhaps for extensibility we could keep that file and any future similar files in a specific directory? |
In that case I would try to make it somewhat obvious by whom (so mod-managers) it's being managed in the naming or by adding a README, Honestly, I'd just stick to the single |
Agree with this, even tho I get the idea of making folders, I just overall would like to make the least amount of changes to the mod folder/files but yeah |
Aight, so are we in favour of a single 👍 / 👎 ? |
Lovely! I'll however leave the issue open for now, and close it once the author file is implemented into Viper. |
Just to be clear and that everyone is on the same page. Using
|
Instead of reading `mod.json`, we now build Thunderstore mod string using `manifest.json` and `thunderstore_author.txt`. The old method for reading is still supported for now but will likely be replaced by a converted function in the future. See also discussion in 0neGal/viper#165
For FlightCore this new standard is now implemented in R2NorthstarTools/FlightCore#133 thanks to the changes done in |
The bb25cfe commit more or less implements this into Viper, however currently it only makes the file, actually using it will be made soon as well, however it was less of a priority. With that I'll close this issue, feel free to re-open if anything else warrants discussion or similar! |
* chore: Bump libthermite to v0.4.0-rc.1 And update broken code in FlightCore accordingly * chore: Bump libthermite to v0.4.0 And update broken code in FlightCore accordingly * feat: Parse TS mod string using new standard Instead of reading `mod.json`, we now build Thunderstore mod string using `manifest.json` and `thunderstore_author.txt`. The old method for reading is still supported for now but will likely be replaced by a converted function in the future. See also discussion in 0neGal/viper#165
This is a bug that might not be one yet but will become an issue in the future. Basically, the Thunderstore
manifest.json
does not uniquely identify a mod as it lacks theauthor_name
of a Thunderstore mod (as well as the namespace but that's not an issue in our case).This means if two mod authors both upload a mod with the same Thunderstore mod name, Viper will be unable to tell which author a Northstar mod belongs to.
I was in the process of updating FlightCore code base from writing Thunderstore mod string to
mod.json
to just putting the Thunderstoremanifest.json
into the mod folder like Viper does when I came across this.As such between Viper, VTOL, and FlightCore, we probably wanna implement a unified solution that uniquely identifies a Thunderstore mod and is used by all 3 of us to ensure compatibility between mod managers ^^
EDIT:
For example, looking only at
manifest.json
you cannot differentiate the two mods when ignoring changeable fields (version_number
,website_url
, ....)The text was updated successfully, but these errors were encountered: