Add RunContext support for script outputs

[sasial-dev]

Resolves #667

This PR:

• Introduces a new field in the project file: scriptType which has the default value of Class (in parity with previous versions), but can also be RunContext.
• This is then passed to InstanceContext from the Project struct.
• This then changes the RunContext in the lua snapshot_middleware

[Dekkonot]

What happened with the submodules?

Otherwise, I think this is a good idea. I'll think on the best way to pass project config to snapshots, though I imagine we'll end up passing arund a reference to a config object, so if you want to assume that's the appropriate course I wouldn't object.

[sasial-dev]

Thanks @Dekkonot! I'm not sure what happened with the submodules, as they keep pushing up and I don't want to force push after each commit (like I did with the first after I saw the submodules) I'll remove them at the end.

Okay, I'll have a go passing around a reference.

[sasial-dev]

Okay, this is ready for a review. Please do let me know where I should write tests for this change, as I'm not sure how far I should extend them to.

[Dekkonot]

I want tests for RunContext! Specifically, tests that check it works at all, if it builds properly, and how it works with nested projects.

Anything else you can think of would be good too. It doesn't need to be too extensive since it's not touching a lot of the code, but we want to have some checks just to be sure it works as expected.

[sasial-dev]

@Dekkonot, I went ahead and switched back from ::default to ::new for this, which removes the diff for the snapshot_middleware, but some sections (IE: instance_snapshot) used default in the past. How do I determine what should use ::default and what should use ::new? Should I change it back in places that were originally that?

[Dekkonot]

How do I determine what should use ::default and what should use ::new? Should I change it back in places that were originally that?

As a general rule, you should be using ::new whenever possible. There are a variety of standard library functions that work if you have Default implemented, which is why it's suggested if you have ::new anyway (it's basically free in that case), but ::new is the constructor you want almost every time.

EDIT: This applies mostly to literal uses of it, mind you. There's cases where you might want to use ::default just because it's a guarantee you're getting a 'default' value for something.

[sasial-dev]

Sounds good - thanks for that! I'll change back to ::default in places where it already was there :)

[Dekkonot]

Like I mentioned before, tests for building and syncing are what we'll need for this, but otherwise it looks good.

[Dekkonot]

@Corecii Just to ensure we aren't misinformed or out of touch, does the plan above seem sound?

TL;DR is that long-term we would move towards preferring RunContext over the two distinct Script classes. This change would add a toggle for people to use now to use RunContext before we changed its default in the next major release.

[Corecii]

That sounds reasonable to me. I agree that we should only change the default behavior with a major version increase since it's a breaking change.

[Dekkonot]

Okay there is a hiccup with merging. I don't have the authority to bypass that CI check so it can't be merged.

[sasial-dev]

Thank you so much guys!

[fewkz]

Is there support for gradual adoption of RunContext for individual scripts? Mixing Classes and RunContext together? Considering RunContext and Class have very different behavior, supporting both only makes sense. I think via a .rc-server.lua and .rc-client.lua extension would be best.

[sasial-dev]

You can use nested .project.json files to gradually adopt RunContext. Or, just keep the existing behavior of class scripts as it is not a breaking change.