rojo-rbx / rojo

Rojo enables Roblox developers to use professional-grade software engineering tools
https://rojo.space
Mozilla Public License 2.0
947 stars 179 forks source link

Support gradual adoption of RunContext for individual scripts #791

Open fewkz opened 1 year ago

fewkz commented 1 year ago

You should be able to gradually adopt RunContext for individual scripts via a separate extension specifically for RunContext. Only being able to choose between all scripts using RunContext and all scripts using Classes is limiting considering there are specific behaviors you might want in Class scripts and RunContext scripts, simply changing from Class to RunContext could break games that expect them to not run in certain containers, or gears where LocalScripts only run for the person who is holding the gear. Having a .rc-server.lua and .rc-client.lua extension that uses RunContext would be best.

Originally posted by @fewkz in https://github.com/rojo-rbx/rojo/issues/765#issuecomment-1742132747

nezuo commented 1 year ago

Though not as convenient as changing the file name, you can gradually adopt RunContext scripts using .meta.json files.

sasial-dev commented 1 year ago

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.

Dekkonot commented 1 year ago

Our expectation is that people will likely want to use one or the other for their projects since that's what people have wanted from us. Like has been said though, there are mechanisms you can use to convert a single script to the other format or even an entire tree using a project file.

fewkz commented 1 year ago

Though not as convenient as changing the file name, you can gradually adopt RunContext scripts using .meta.json files.

Does this work? I thought the reason RunContext support was added was because meta.json didn't work for modifying the RunContext, I don't know if it was fixed

fewkz commented 1 year ago

Our expectation is that people will likely want to use one or the other for their projects since that's what people have wanted from us.

Would still be nice to be able to mix and match easily in the same project with a set of extensions for RunContext. I would love to use RunContext for scripts since they can run in any container making it easier to write code without having to put it in the right place, but I have so much legacy code that I don't know would work if I were to switch everything to RunContext, being able to gradually adopt RunContext without having to make a nested project every time would be nice.

sasial-dev commented 1 year ago

Though not as convenient as changing the file name, you can gradually adopt RunContext scripts using .meta.json files.

Does this work? I thought the reason RunContext support was added was because meta.json didn't work for modifying the RunContext, I don't know if it was fixed

It was fixed in 7.3.0, I just felt like it was deserving of native support.

kennethloeffler commented 1 year ago

Our expectation is that people will likely want to use one or the other for their projects since that's what people have wanted from us.

Would still be nice to be able to mix and match easily in the same project with a set of extensions for RunContext. I would love to use RunContext for scripts since they can run in any container making it easier to write code without having to put it in the right place, but I have so much legacy code that I don't know would work if I were to switch everything to RunContext, being able to gradually adopt RunContext without having to make a nested project every time would be nice.

As some of the other commenters have said, it is currently possible to gradually adopt RunContext scripts using Rojo today using meta files, it's just not the most convenient thing to do.

We went with emitLegacyScripts because we think that in Rojo 8, "emitLegacyScripts": false should be the default behavior (a breaking change!), and we'll support the legacy script types under different extensions - sort of like your issue here, but the other way around. emitLegacyScripts and future file extensions in Rojo 8 (probably *.legacy.client.lua and *.legacy.server.lua) give us a solid path for migrating older projects and will support mixing and matching legacy and RunContext scripts. It's a bit of a hairy compatibility issue. So, emitLegacyScripts is more of a stopgap solution for people who want to adopt RunContext right now (especially for standalone model-like projects), but we'll make this situation better in the future!

Adding new extensions isn't to be taken lightly - we basically have to support these forever, so we must consider carefully what we want the future to look like. In this case, RunContext is Roblox's new recommended best practice, so Rojo should make that the easy default path, while still including legacy support for the older script kinds.

Dekkonot commented 11 months ago

To add onto this, we aren't going to do the above soon and in fact don't even have a ballpark estimate for when we'll do it. We anticipate having to at some point in the future because Roblox is probably going to back RunContext going forward so having an easy way for people to swap between the two is important. Rest assured though, our current plans aren't to just suddenly force people onto RunContext by releasing Rojo 8 in like, the next release. This is a long-term plan.

The reasons provided for why we didn't add a new set of extensions are valid though. It's not something we want to commit to supporting indefinitely.

nezuo commented 4 months ago

With #909, you will be able to specify .server.lua and .client.lua to turn into their legacy counterparts even with emitLegacyScripts enabled. You can then add a new extension for RunContext scripts.

For example:

"emitLegacyScripts": false,
"syncRules": [
    {
        "pattern": "*.server.lua",
        "use": "legacyServerScript",
        "suffix": ".server.lua"
    },
    {
        "pattern": "*.client.lua",
        "use": "legacyClientScript",
        "suffix": ".client.lua"
    },
    {
        "pattern": "*.rc-server.lua",
        "use": "serverScript",
        "suffix": ".rc-server.lua"
    },
    {
        "pattern": "*.rc-client.lua",
        "use": "clientScript",
        "suffix": ".rc-client.lua"
    }
],
tapple commented 4 months ago

I found one place that RunContext: Legacy is necessary: TestService

Scripts inside TestService should never run when playing the game (F5), only when running (F8). However, if they have a non-legacy runcontext, they run when playing the game with F5. ModuleScripts and LocalScripts inside TestService don't run at all, even with F8

This might be an upstream studio bug with TestService

Dekkonot commented 4 months ago

This isn't a bug, it's expected behavior. The point of RunContext is that it allows scripts to run anywhere. The Legacy option doesn't run everywhere and only runs in specific locations (Workspace and ServerScriptService).

That said, I think there's definitely an argument to be made towards changing it, but like you said that'd be an upstream Roblox issue and not something we can handle.