AP-Hunt / FicsitRemoteMonitoringCompanion

A companion app to the Ficsit Remote Monitoring mod for Satisfactory
23 stars 9 forks source link

Update 8 batch fixes #20

Closed featheredtoast closed 10 months ago

featheredtoast commented 10 months ago

General updates for U8 and recent(ish) changes to FRM:

AP-Hunt commented 10 months ago

Thanks for raising this @featheredtoast, I appreciate the effort you've put in. I'm not in a position to be able to test these changes at the moment, but I've reviewed the code and it all looks OK to me. There are a couple of magic strings it might be nice to convert to constants, but nothing that should prevent this from being merged.

Have you been able to test these changes for yourself at all?

featheredtoast commented 10 months ago

Yep! I'm actively using this branch via my downstream project here - https://github.com/featheredtoast/satisfactory-monitoring -- that's how many of the changes came about.

RE: magic strings, can definitely refactor, I've just been sitting on this batch of changes for a bit, just wanted to get the ball rolling here.

Also re: go mod names, this is in response to @surdaft wanting to include and extend the companion code (I believe he may have contacted you on linkedin or somesuch about refactoring but this method should cause much less disruption). As far as I understand that's how to allow modules from subfolders to get included per https://go.dev/doc/modules/managing-source

AP-Hunt commented 10 months ago

Reading the docs, I agree this looks like the right way to expose it as a module to others. Given that you're actively using the code as a way of testing it, I'm happy to merge. Thanks again for your hard work.

Unfortunately I won't have access to a Windows machine for a few more weeks, so I won't be able to cut a release until then. If you need one before that, I'm happy to give permission to try it for yourself (see the Makefile).