search

rojo-rbx / rojo

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

Syncing large scripts breaks rojo serve #199

Closed tiffany352 closed 4 years ago

tiffany352 commented 5 years ago

Repro steps:

  1. Make a really big lua file
  2. Generate a place file with rojo build
  3. Open it and try to connect Rojo
  4. This error happens, breaking the connection: 
    17:08:12.823 - [Rojo-Warn] Rojo session terminated because of an error:
17:08:12.824 - [Rojo-Warn] Plugin_1997686364.Rojo.Plugin.Reconciler:65: Cannot set property Source on class ModuleScript: String too long

I'm guessing what happens is that Rojo is reading back the source of the file, which is truncated, and then comparing it against what it should be, so decides it needs to be reconciled, which then fails?

It seems like this failure mode might be new in 0.5.0-alpha.11 since I only ran into it after updating.

Kampfkarren commented 5 years ago

Isn't there a character limit on setting script sources inside another script?

tiffany352 commented 5 years ago

@Kampfkarren Yeah, Rojo can successfully build place files with larger-than-allowed scripts. The issue here is that doing so breaks rojo serve.

The limit is something like 262 000 bytes. Edit: It looks like reading from .Source doesn't actually truncate.

LPGhatguy commented 5 years ago

Wow, this is a fun regression.

It happened in commit 075b6cca30f12bc5d087308e15114b397d7e4ae5. I refactored the code from reading, comparing, then writing to just writing and letting Roblox deduplicate the property assignment.

This looks like an example of where that solution fails. It's not possible to regression test in Lemur, but would be a good test to add as part of migrating to run_in_roblox/robloxcli, and I'll make a note of in the source as a caveat/bad edge case.

I wonder if we can push the systems folks at Roblox to raise or remove the string length limit?

Nolij commented 4 years ago

I'd highly appreciate if this was fixed, the project I want to use it for has scripts too large for serve and using build is highly inconvenient.

LPGhatguy commented 4 years ago

This should be fixed in Rojo master due to a change in the way that the initial set of properties is applied. I'll be releasing a fairly stable alpha today.

If this is high priority, I can try to backport a change to 0.5.x and release a 0.5.5 patch. Otherwise, 0.6.0 should be around the corner soon.