CubeCoders / AMPTemplates

For the AMP community to share Generic Module templates.
79 stars 239 forks source link

Update Stay in Tarkov for 1.6.5 and futureproof Github integration with new setting. #795

Closed dzlockhead closed 1 month ago

dzlockhead commented 2 months ago

Added 1.6.5 SIT and 1.8.3 SPT. Removed incompatible SPT versions. Added SIT Release version and made it as easy as possible to understand.

Greelan commented 2 months ago

You need to rebase your branch to remove the merge conflicts and so we can actually see what the changes are

dzlockhead commented 2 months ago

Okay, NOW it should be good unless I missed something else. @Greelan

Greelan commented 2 months ago

I find this confusing tbh. Why is SITReleaseVersion needed?

dzlockhead commented 2 months ago

It's used to choose the release tag on github. You told me that it won't work using just a file name and variable, which is the issue I had in testing. It worked for 1.6.4 but when I tried 1.6.5 without the UpdateSourceVersion, it still tried to look at release 1.6.5 for the file, so I use that to pick the release tag.

On Mon, May 20, 2024, 6:15 PM Greelan @.***> wrote:

I find this confusing tbh. Why is SITReleaseVersion needed?

— Reply to this email directly, view it on GitHub https://github.com/CubeCoders/AMPTemplates/pull/795#issuecomment-2121306604, or unsubscribe https://github.com/notifications/unsubscribe-auth/BE3Z6XNHOZNEPY6MKBIS4CTZDJYX5AVCNFSM6AAAAABIAMOKT6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMRRGMYDMNRQGQ . You are receiving this because you authored the thread.Message ID: @.***>

Greelan commented 2 months ago

If you are specifying the full file name - as you are - then just use FetchURL. My comment was related to using GithubRelease where there was not a standard name format between releases. Its unnecessary to adopt an approach that requires the user to set two variables

dzlockhead commented 2 months ago

Makes sense! Modifications made and should be good now. @Greelan

Greelan commented 2 months ago

Just be aware that this will break existing instances until the user toggles the setting. The alternative is to keep the existing enum values but have separate update stages for each (based on conditions). This just might become a bit unsustainable as new versions are added in the future.

dzlockhead commented 2 months ago

Just be aware that this will break existing instances until the user toggles the setting. The alternative is to keep the existing enum values but have separate update stages for each (based on conditions). This just might become a bit unsustainable as new versions are added in the future.

Do you think there'd be a better alternative?

Greelan commented 2 months ago

I suggest taking the hit now so that it is sorted for the future

Or get the SIT devs to name their files more helpfully xD

dzlockhead commented 2 months ago

We can only hope. Let's go for it and take the hit now haha.

Greelan commented 2 months ago

You've got the SPT AKI Version setting in there twice

dzlockhead commented 1 month ago

I am not sure how that occurred... It's been removed.

Greelan commented 1 month ago

I am not sure how that occurred... It's been removed.

You haven't committed the change.