search

Danacus / node-factorio-api

Download and update mods from the Factorio Mod Portal
3 stars 2 forks source link

getModsFromSaves() returns jargon with larger mod sets #2

Closed Danielv123 closed 6 years ago

Danielv123 commented 6 years ago

https://pastebin.com/SHx2JJpc

The code used:

factorioAPI.getModsFromSaves().then(list => {
    console.log("Mods used in save:");
    console.log(JSON.stringify(list))
    console.log(asTable(list[0].mods))
});
Danacus commented 6 years ago

I'm going to be very honest with you. The code for getting mods from a save file isn't actually mine. I found it somewhere and it worked (I forget where I found it), but it might be outdated and incompatible with 0.15. So I have no idea why it's not working and how to fix it. I'll look for a solution when I have got the time to do so.

Danielv123 commented 6 years ago

JARG had a lot at the error and said it looked like it was unzipped and then just returned the data as a string, which would explain why definitions from data.lua has become muddled in there.

Might help you debug.

Danacus commented 6 years ago

Fixed in #3

BrotherWomba commented 6 years ago

The number of bytes between mods changed in Factorio version 0.15 from 4 to 8. I made a pull request correcting this and also corrected a little offset bug when reading the name, so it should no longer be needed to trim the name or remove non-ASCII characters.

BTW you might have found the original the same place i did https://forums.factorio.com/viewtopic.php?f=69&t=35644

Danacus commented 6 years ago

I merged your pull request, thanks a lot! I did indeed find the code from AlyxDeLunar. I didn't test anything or make any changes since 0.15 released, so I didn't know that it didn't work anymore.

Danielv123 commented 6 years ago

Sounds like we need unit tests, but good job on fixing this one 👍

Danacus commented 6 years ago

I did initially make unit tests, but they don't work very well, because of timeouts when it takes too long to download a mod.

Danielv123 commented 6 years ago

Using mocha, you can extend the timeout for specific tests. See for example line 104 of https://github.com/Danielv123/factorioClusterio/blob/f3bb8c58e56865c7b8643cd0d6454975d6c092ac/lib/fileOps.spec.js

or line 39 of https://github.com/Danielv123/factorioClusterio/blob/b847b1a934a73186b6ca455bd65f09f9c779af1c/master.spec.js

Danacus commented 6 years ago

Thanks! That's really useful.

Danielv123 commented 6 years ago

Using the latest package version on NPM (0.1.28) I still get the same error as when I created this issue. This is on a map generated in 0.15.37.

Should this issue be reopened?

Danacus commented 6 years ago

The last pull request was merged 1 month ago, but the last time I updated the package was 2 months ago. I might have forgotten to update the package.