search

screepers / screeps-launcher

Launcher for the Screeps Private Server
MIT License
129 stars 34 forks source link

Update config.go to include default pinnedPackages #36

Closed kalgen432 closed 10 months ago

kalgen432 commented 10 months ago

pinnedPackages now defaults to the 4 necessary to make launcher work, per #34. Pushing this and triggering a new build of screeps-launcher also seems to address the failures to download yarn from github and address https://github.com/screepers/screeps-launcher/issues/37 too.

pinnedPackages: ssri: 8.0.1 cacache: 16.1.3 passport-steam: 1.0.17 minipass-fetch: 3.0.3

kalgen432 commented 10 months ago

If it helps to test it, I pushed a rebuilt image with this PR: kalgen432/screeps-launcher.

AlinaNova21 commented 10 months ago

I'm not a fan of hardcoding this, seems like it could potentially cause issues when the packages and requirements shift in the future, it might be worth adding a note and link to the issue in the README though.

kalgen432 commented 10 months ago

Thanks for taking a look.

I don't know if this is what you meant by hardcoding, but I think this PR specifies those packages as a default, and any pinnedPackages: section of their config would override it? That is, adding this PR doesn't introduce any new issues or remove flexibility.

For first-time users: With this PR, screeps-launcher will work with any previously published config out of the box. Without this PR, those configs are broken unless updated with a pinnedPackages section.

For longer-term users who have an old pull and and old volume that's already built, it should keep working both with and without this PR.

For longer-term users who have an old pull and are trying to build a new volume, regardless of this PR they have to do work to fix it. Re-pulling the image is enough if we use this PR and keep the image defaults updated with current pins, otherwise adding a pinnedPackages would also work regardless of this PR (with or without it).

I'd tend to prefer both updating the defaults to be working and updating the README to leave more breadcrumbs for future breaks to suggest re-pulling the image and updating pinnedPackages. But if you don't agree, I'll abandon this and prep a PR for just the README updates.

AlinaNova21 commented 10 months ago

Hmm, good points, I guess the README can be updated to mention it and this could be merged for now.