From 37e44e474ac9af552a845eb94f5b20095b97a2ce Mon Sep 17 00:00:00 2001 From: astrid Date: Mon, 15 Dec 2025 18:45:59 +0100 Subject: [PATCH 01/15] feat: support `name` property in meta and model jsons --- ..._syncback_util__slugified_name-stdout.snap | 14 ++++ ...lugified_name-src___Folder.model.json.snap | 8 ++ ...ied_name-src___Folder__init.meta.json.snap | 8 ++ ...slugified_name-src___Script.meta.json.snap | 8 ++ ...ugified_name-src___Script.server.luau.snap | 6 ++ ...ied_name-src___Script__init.meta.json.snap | 8 ++ ...d_name-src___Script__init.server.luau.snap | 6 ++ .../input-project/default.project.json | 10 +++ .../slugified_name/input-project/src/.gitkeep | 0 .../syncback-tests/slugified_name/input.rbxl | Bin 0 -> 48527 bytes src/snapshot/metadata.rs | 14 ++++ src/snapshot_middleware/json_model.rs | 1 + src/snapshot_middleware/lua.rs | 8 +- src/snapshot_middleware/meta_file.rs | 77 +++++++++++++++++- src/syncback/file_names.rs | 54 ++++++++++-- src/syncback/mod.rs | 15 +++- tests/tests/syncback.rs | 3 + 17 files changed, 231 insertions(+), 9 deletions(-) create mode 100644 rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__slugified_name-stdout.snap create mode 100644 rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__slugified_name-src___Folder.model.json.snap create mode 100644 rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__slugified_name-src___Folder__init.meta.json.snap create mode 100644 rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__slugified_name-src___Script.meta.json.snap create mode 100644 rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__slugified_name-src___Script.server.luau.snap create mode 100644 rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__slugified_name-src___Script__init.meta.json.snap create mode 100644 rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__slugified_name-src___Script__init.server.luau.snap create mode 100644 rojo-test/syncback-tests/slugified_name/input-project/default.project.json create mode 100644 rojo-test/syncback-tests/slugified_name/input-project/src/.gitkeep create mode 100644 rojo-test/syncback-tests/slugified_name/input.rbxl diff --git a/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__slugified_name-stdout.snap b/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__slugified_name-stdout.snap new file mode 100644 index 00000000..b50c2448 --- /dev/null +++ b/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__slugified_name-stdout.snap @@ -0,0 +1,14 @@ +--- +source: tests/rojo_test/syncback_util.rs +assertion_line: 101 +expression: "String::from_utf8_lossy(&output.stdout)" +--- +Writing default.project.json +Writing src/Camera.rbxm +Writing src/Terrain.rbxm +Writing src/_Folder/init.meta.json +Writing src/_Script/init.meta.json +Writing src/_Script/init.server.luau +Writing src +Writing src/_Folder +Writing src/_Script diff --git a/rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__slugified_name-src___Folder.model.json.snap b/rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__slugified_name-src___Folder.model.json.snap new file mode 100644 index 00000000..10829e88 --- /dev/null +++ b/rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__slugified_name-src___Folder.model.json.snap @@ -0,0 +1,8 @@ +--- +source: tests/tests/syncback.rs +assertion_line: 31 +expression: src/_Folder.model.json +--- +{ + "className": "Folder" +} diff --git a/rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__slugified_name-src___Folder__init.meta.json.snap b/rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__slugified_name-src___Folder__init.meta.json.snap new file mode 100644 index 00000000..eb336d02 --- /dev/null +++ b/rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__slugified_name-src___Folder__init.meta.json.snap @@ -0,0 +1,8 @@ +--- +source: tests/tests/syncback.rs +assertion_line: 31 +expression: src/_Folder/init.meta.json +--- +{ + "name": "/Folder" +} diff --git a/rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__slugified_name-src___Script.meta.json.snap b/rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__slugified_name-src___Script.meta.json.snap new file mode 100644 index 00000000..c4d203b8 --- /dev/null +++ b/rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__slugified_name-src___Script.meta.json.snap @@ -0,0 +1,8 @@ +--- +source: tests/tests/syncback.rs +assertion_line: 31 +expression: src/_Script.meta.json +--- +{ + "name": "/Script" +} diff --git a/rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__slugified_name-src___Script.server.luau.snap b/rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__slugified_name-src___Script.server.luau.snap new file mode 100644 index 00000000..1a45520f --- /dev/null +++ b/rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__slugified_name-src___Script.server.luau.snap @@ -0,0 +1,6 @@ +--- +source: tests/tests/syncback.rs +assertion_line: 31 +expression: src/_Script.server.luau +--- +print("Hello world!") diff --git a/rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__slugified_name-src___Script__init.meta.json.snap b/rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__slugified_name-src___Script__init.meta.json.snap new file mode 100644 index 00000000..2798b41d --- /dev/null +++ b/rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__slugified_name-src___Script__init.meta.json.snap @@ -0,0 +1,8 @@ +--- +source: tests/tests/syncback.rs +assertion_line: 31 +expression: src/_Script/init.meta.json +--- +{ + "name": "/Script" +} diff --git a/rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__slugified_name-src___Script__init.server.luau.snap b/rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__slugified_name-src___Script__init.server.luau.snap new file mode 100644 index 00000000..ee496521 --- /dev/null +++ b/rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__slugified_name-src___Script__init.server.luau.snap @@ -0,0 +1,6 @@ +--- +source: tests/tests/syncback.rs +assertion_line: 31 +expression: src/_Script/init.server.luau +--- +print("Hello world!") diff --git a/rojo-test/syncback-tests/slugified_name/input-project/default.project.json b/rojo-test/syncback-tests/slugified_name/input-project/default.project.json new file mode 100644 index 00000000..38b17470 --- /dev/null +++ b/rojo-test/syncback-tests/slugified_name/input-project/default.project.json @@ -0,0 +1,10 @@ +{ + "name": "slugified_name", + "tree": { + "$className": "DataModel", + "Workspace": { + "$className": "Workspace", + "$path": "src" + } + } +} \ No newline at end of file diff --git a/rojo-test/syncback-tests/slugified_name/input-project/src/.gitkeep b/rojo-test/syncback-tests/slugified_name/input-project/src/.gitkeep new file mode 100644 index 00000000..e69de29b diff --git a/rojo-test/syncback-tests/slugified_name/input.rbxl b/rojo-test/syncback-tests/slugified_name/input.rbxl new file mode 100644 index 0000000000000000000000000000000000000000..9b91b6947adfa786e8fe096841ffbc8c27dc1954 GIT binary patch literal 48527 zcmb`QdyE~|ec$gcm!wEh5-ExjMM>5bO-qz5N_@()tOxhOC0FKSwYwy3!?ASt-r3zN z-o3NF_b$nmC54u4B`FF-4va>2A90hksoMaJ-84Xe0&e<8Yd31@q<^L8!zkJUX@O!| z%WMm@VZWa-><)_d{hc{@zb5UMz7X-wa%>9>owbpT`TcLBk^(leoz-FxPMsx-luD9 zb$-6(jW5-D<9>UgxwzV?^_qUW5*2oUvXFXI7b&=Jlm5L&*ZAz|iDn&et+SS@ltX)L>+dmjc|za904aE2tNx{S;HKIutG%&WC%26Kl;JXWo8)K}vX`6fhJP(r zvWn!#g!`y2QgF{U{fi-t`>mE|m`l+%VXErVErSZ__S-%0dT%sO?vjynP2zURVG`rM zf2HYlQzcCsi39o_DN?W-gWIWVLN+wp^E+Njrfp>Q2?6P(VA~z~7t$xZ`A#!7xbp_- zs-L=mob+1_uVdA}1(vA7Bf3Z@vHg)su>G{mWV+T(SNyEW`eA_{03Af1@H%&gfTD$Dcr;2#h2w>AzW5L~+ z18O^0>v^4Kt(6ja(1@H6;FvB_Z~)sNgY$lS##?DM>k@4gMUXoPy#{(jKRl(26jX`*z^6NY-RpL(_2gG_-elm0B}^7wD}(FJ zvL2O0}O#Fo~@DnIV)nHP+d>r7aYX-g>Ova}Re4_%Uex}D16P*JllHM`CEW=s0q zRr!u>)@w$tDpWFLz+{Qwv;Jzkk>YHJ!MpatCp>*t0|vRRdb3Wgu0O(tb**}yMLm>a zM=!x0^w!%G$AIBKhO+*HXmbUb^rt~sx^ z96SAW19Uy_s6cq$oa}bR&vY+iM#SxK3893PIj_^HHREyG2z7ReKQ0t4ob$TrDDZkt zU5L+NU9R^S+N4bABSxeuP-1dV>k}q7*VKqEh2L)QZq)dsutbeHf3?13hFvM4FXR;R zj4kAv=e1M#eFpE2Vm>821A>dE$L(m?89u*ZWZXFYtnhTh7pKPkPCFT$d>mHYQ1fZQ zu!M_Kr#rPq(`(0p>`}mO_x_xK#HEW<)3ruJc4kfNiEPHtN7zd&xZ(%VMH<#kMWsJiTTu5?#w$#kbOG(B@++6@xInGSih)$*@}BeThl=b713 zSFon(AduBXrqQ5u9Z%DY_!?#c+a%(33Y79*!Ov;B-z=XHZYCjOG4_1EX&^fQsTwQ+B?=4u8 z_bV^b%@K+za1usmp8%;8JO~(7ni*+0Qk(6qHo6xU7E~#86(>q^2T2Iik)Eqv4>j5y zxp5=ZjyvK3h)oMPp>WBZdgSZx4$>nQd9K;k4Cr+)hJWp32+ul8?BfiD2dsrr_o{wQ z0xVT3f8$U8 z^k2X6XMgrzsH!`(K>)i!U{;*iZr~tLH^y7&#l9H-y>z_Uue_?dNWm^du=Yz@8_^uT z)u(>io6{51^&zqGo*0P|-f0WiV6OnYz~J4G2RODECLMVz7Flyatcth0%lifAG~$fAqxL|Kz{@m9HNEH@^=P?tqvaVPf2@ z0ne_^zovCiDrJJA8z8`9>LGS+ZN(!r<_y&UmM{?D6gkj9-0%aO@H5%;T8#?}mzv(S zk*{llPUDCO$-EdOGpIh~IlqzA+U=o0GfABE>#NW#KF z-f6XLznc?`;D0@ra3cT`xr3fBf8!g!^5yS+@B3f=<3Iir$Z-cfh!qTa^s7k-xdnw{p}V?s2)*l>-} zYv{IAUnh*C*$dgKJ|}ibm5Y5Ns~4*2c!xU>Aj{N1;H_6)dDULj!5s)7kT(!G05spo z&6uIr@vnFhxym9=h~*o3BIbF@q1}ccLyFsqV+u2jkic(`N<&9 z1_smjxNZlygxlHL40Jm;{sJ#Bcenom%usr&JG$EQ&oviygI3|+xT>pRkn}%i{aQmc zsjarmLS8tGbVr*&V?W>zR%x@sHnaQvli^DcXVVqGT=9_yTADccXz@xfI z!F|X=mgcKsq~Km8K7?Za`=-9N@I(9G&9NHY$zN9a#Yo7!$Qn~fS z|Mx{Aw@acJrbK5vwk;IRG(Z-_0Jl#EOpDL>o#yLW&Z@OWTg}DxvhJ}k^D_>?Fz}!* zQgA;KaW_T}M;yr6lauvkB+GcSEy?j_*E@l%peND5EGW)h+~OHRn$5Em@bFjoy-LL9S6?J?lBW+q)uB z!U=5w1D%kNN}Wh2L61~^FS&2y%$yJCR|^exhuEl>f*eIuci&gmE|Do?m$nSYF5M9q z5a{mL=I7GdbV%4AW|2;@B)xMk%RFLbn773g zFb^cn{6aDFgw=A)BT<5RTfhdH2VTIu8vy~w%rj?k_a0_BN1zPnwtxc8fuuQqwU~3* zTaRvh(`i54*t>m{88sMWJA;~Tq+Y-_SXnYRi zFOZ<14eH1iut6OGFVK-YEeA|{b*pQt=Ph?PX~6emn07l)V6mSup$>Q70;FlSS3Viy zg00#TwxN9hHs1n`*c$eCCa)B01V<^S5hO}z#1^nYjQ}sui0kQqW2zZ^KNap@&YC|+ z*x4xAHs=DefuziT+dnHNn>lnjvXLl3wk=?TWCJfCoBpG!i}Yi#G1qtKoSdMcfN^ouq*dXV?3pjT}4q*CB_RiahrCCXEdk zw8|Y7gI-V1?hQF4&@i@D?m>%-s243CLblo@Ed^ zby9E-VuRN-P_5CrI|{J%AM-ofOcE^!=`PprzVY(QGjDwNyT462Zr_9mqoQ@%4y zX*R}IbHklk)N#99@};R~Yis^$?*f*GQJ5yeGV^g#_#jUllrTC|74t-owV+IG&$AE4 zJ|-~XUu&b;_zqAp{H(W-Y*uimK2#cwI@!;#JQn9q-pOVG1W=D7aB(F@gQN)3O=BWN zziGeQtzgC59=k;cR?xcg?ZUg2c!x)9p5!S6ql zJqpCrjZOVfAAX4Q49HVBJ?+@-@$5lDB5?uoNjJBUDGofV$GW_6t=@W_y^g)+_ZSB5 zE(KPVi*?B)K5cY_Z5!u|Do~yEtH4whW7c*N&u~C29jV;Vzb3&%wo62!e**Lm(&G@U zfznCAFn=+1aJKbjI~9g1pPyi z-!I_WG6K|oG(G zQxqD0+6v!>aE>BOYlf7NaN?FNE!0FmOVsb>DC3`Q0_Un@NDnPA-w*4;HfF{?`-*)cc+RUYwY9H)aZM3tq3Jck9U$s>Hw{&vsJwHb(_Cz} zYpo|VU%t>5=^ooZMhGEPzzwxjzzohMwA9mcD&3JAH$osG+;$7WodC1er7ub3dr<#V zRZ?&taCB}4rn5`Uh2AS;eot$oq=2|jkP%qmX1$!t1 z+d0*~+U)q|ZsJ69VPUoFxqXd0%R-9bYoFBYidWZ@zj{Eje`h6zjH8iGBE_@or>?K~ zT|JUDuDvH-yX%gVsG1FM!EQu|(5J5VG)FO7pQ*JMy*dBL(Zd8BLwA+|ta{R4G|cUI z>nmURn!RAa?OZ5gO9(&JZjAi$JKr&RLJB}qumw<*r+HG7IBH-tj^atdPGHpPwC8Ih z(#le^epap1rhhLM$eJ9keY)k(Yk&AsP3I+a!6I5q|N#Zp~t@Qt#5sss#o=M()R(*38DH{$*_XY;H0Er z7(A>veOXp@M2jQ@q4`buWurQt1HG^?QCk~=k0ZyQJ$Crn!-v7Uoup-~e+>0rx|763 zs=7$QE^Ej){t!@Yc99tlT^v|^hE}|h-Mx(YAN24zB zKu}FT@w3ImGh3s|8z&x#62#jA28MH&c;E%ZyRHv7CZ2}J$0}s}2WT0#ZPo>B14*;} ztzx!`$>rEaq6FKvfDN(@ynyYheojIV;FxU_o%;_E$%YN^Whl4#7f=o)P5D}ya<@mL zmp5y`-Ib+Ut7m!%Go%YYQ*AdCAy7_7NSI+)t!qo5KOQq5DF72s;y!RlAL}&hS4@3a z9Wy98FekR9qCegu9%pB~pm zx_LYK@6mBU~Hub_>1dMO zIOl0snvtVw=94War)pZ|)RQ`AYgcPVgsPcOh`7BERl}wRx@A7g6V2MP-)?9tYt$N8 zHlOl~K#`L*ozzq}6*7ZD^AWfkI;j!dO#02R!FVGn0c!c=`ILjpsWlgQ!h#MMyw0YYB_{|X-S`gygArW9|I#my? zXH)n$xsoGxP5NLPK*nxoz1qTBQtj=#E7Nhc^I4QcZ>f*P=rcv+yYg9$x)S@xm1FzQ zC+1wOBbSMZ2|rq5vSBE(c}S0KKA)I#t0ChMqrWXNRedA{yC5d+Byix=p;1WWyX|!C zsy3^%!x)5V+Ak2vZ|Ss0aEMzPegvLRy*#I@HH+AQqtELIM9zd+7=dOTfqZY8(X)Fd zdnn)o(0tz}5^5^lUhI#c<`Xjc?VZtKeo_7L?eOE^+RWPCh}!U>2pYLk2vy99SVUK( zq-)VL=ZyjOnD5oIarnZ(%qMcknX;Y4?6U4vEJZ{cDv89>TM_NEZU0(KJ0Hlus{E6| z3wBv%Fkm5CO^r-cX?q`=<;S4d~tS)SC6qnaf%Pd6cmTTV{bp07+ZK(`k!vN1NO@DyIb`N@&3rut6;VFVF%`YePbC z58zl0EFNS}>dW$uP#NBBAqBhxN%MZDn0Fkl9Pdb!;N2FmLEeEE@a}eKfMebnhPcNF zW;sWo4Cl6h0?vV?Isbey=dAIS;~a@X&Rt%Bvj#H0st#&-+Sd(A9hTy3dvZgRt8`ih(Cv!4RyD&TN*$q(3-Y)w(#f!P!QEN` z3yDAi%P9_y2DH$EKKq^&Z(B)UNo!rI%-D;bZyf2 zMtND0w24m@n>fl`&%}vVj2%e9HY9N2F+HfR(V(R>-m=2)q~K2eDs%?;qnV`O4&a!c zo|y_yehhc1)f&|srW!o0Dw1;ZjSJ&z)W4Yv^CMT=g{JG{Hey_x~m28&tacg zPGY-`2HvvkF386uf^Cw5d&wFur#s$)PE7QeD}&rX0W1n>56w#LTAPLKksCKc8>s60 z#1bi>j3t7n>IIP#NZPDkOq*4fLHt|e##!yn&>3`#qdEi0m~ifacr;)=J9G^Ii9+kj z!b{i-xfj^WVg3B1E>iF?;8^{zRd{G_wzosRjLvL%1v&$g*4gW6ow=O_e3erH5+zh% z3mE9Kl%CuX0*)^^5r%eIuMUQRI9l3O(TQQnO*%I`f+nK!G()?=5zl_78*PHYv8!3l zAVQ6Z1m^hS44&Qt7bgXK!N-}B+8Huew|neBu)BQG3i9ka-<_s zf^=KJ21y5AK>7jwY&8_@1{{-)rzE7$hG$D>X-B3E?Y4{pTL+S+{maF)(}r@iBT<5O zTfjitiA4-}NWmuHG(m?1vg9n6l}NEJ=<(+)j!-*HpEY8#2vo? zjkQS3eJVV#8YCHV8;ca&29m|>nbo%LQt%k7X#v0&_4V_*NWqg97-pn1xf?^w1HL>l zHg;m-`0>$0Pmdira_IP}6XS=*CdQsV^z`KT*GD1l_iP4bly~ zfNpn61~{ghm~7|&X0|eLI}BoFn773gFb^cf{N@jenJ1=}V;+eT%-aGs$UN`@=7~Y{ zQlwx9V8#sW5Y;rXZ#BX6QT&WCl7c;aiq%Au(TsgP#6T?}RYps;lmaaQNoi@*8^v0} z;L2$Ui4t0}1#D1Dzzeig)z3-)2{s>%LacoAueIc@I*0 zS=jAb5u<PTCny5oA6d!7{9?>LLZZz^-RORbdiOL4tTT%1c>BSDIsMG7gSCj7z{C zP%|mG6MUh?VfVC}6x<6O-Np-~8Vu{!w$|9rd)HK4TnZ8DfG$$72QdclQC-gJkX=vj z6k%1L=jD(xduX$DqxA#L+YRLuf_d8`&ck);ksE*a`4vptotfBnPxvc(9x%0m-Sq;X z+jI?ha1~3VwWQ!a!03CbD?+sAeNtPd)2aw}LucC>dk+dR#P)d%R);&2g1adOLRpN} zbFATmC4HS&&Yo{X$e8ng{e=a0%YIcqxJFdQAIX}^7x*KPv_C$c_D5=vEXE^Z{Lyc% zF6%IvX5BD}v6$w$uEK4U^<2fJ9En2H&%)QWM(RYmIYRE+bPcc?e#UZ0!To%SwU1s0 zyn!m)(;-<#Rkox8RRKw>>hGjg<&sZNP>y^gN|0|0C=3M&`M?XvcjE@&nEdVf5RtD1 z1Ne6PP{7<(kmGs1@WzR77utH8#7PKKBg zG?0Q}GF&IR07;wd%W0FX3gnVO8JC*oJx=4Rovz<;o(p$P{0U*pnK%+9OxzZ*K@$gF zVB)Tw0FF)kc72FUoHx&9V+!bEJ8>0cb1#S~K++1DPAkauv_lfX)51r4&F+N0`*%4# z00Qf2$Q=YAK@Wo|zO=d=ANFE0Kc*j!=pqICfgaEmzVqlMjzzh+vZ&Xcd9SFGj_%c) zu5~qWZUk&l3U=EDjhnXx=yuNMeSg^wRz}_t7F86=G~15MTak>rK1w-+v8bHr$Y7JX z+oJ*8VSoeP=jD$5fWu!r_8aSXwJU~-?uGUQdvwF7<2nvvs3jJI>F}7;k*~b}{`=V& ziv&^4k(l8dPIW`s$xwDFe-YAx!O$^co{yRpGRQp)nO91zV|>;a7WT^;*?FhO*U1h2!G0=S5@Wuy0X7TD5Pz z$tn+82ZoDm8PrU@L}(Wm>h;zt?}v3_9)3#yPYNEO0rZs!>9I|{Bd%kOD%^MM$hw^e zvf+?FmOZDp6VN?5QjGGjGg7b@DOMp>qIn}?L2>SY7(o`!tYB_g`T7iR?oO<=c{+o) ziFDDj(PvhGh!hH()0~feNmnkcFoy|EAMTQ2o#lWXOtaAo$VYiO)zk#m}Gz{ z-*@a-SA)k;n3BnW6nuogm=5Nb9(E!3Ei5d&xUle!DLdO`R|kq~F9ic}wcV-y`j(Yc z-yGDe`X7typWk__nXTTEdioV@w2C`?`@S}Jv4J)h4C}H%kJ__%9d~1+D=VIMuj!R= zVMxjv9h^EMW?-(ZnJqm=phbi@{N;BcdIx%37&l-$9O2xFg{sOu8D`U-5nbj+(jEH4 zjy%ntGy2T@(E|8Ej8&D5TY|}WgC~008WoIIfuu*P7t*6ucM^#R(+LeC^fq)~Z&P*l z47rWO$mqK_{s1u&g+n!Wa9|4P4tRmlxuFDbtWk5XD(5ZJD7GqKq9(-3sL2*n zpe7(`HT_mvO)m2Um~zY`QG$6}zy_HIUch`+KPMCdj+uwcaB{5>cBa-{%2E!#4COZe z0?L7;DgSPo@@EBlMi(h~9Ffhs%=^T{4H(CdJ$>}V@uSBN9XWD%^3d@Uqt6`r)TyVB z9U41z?D*LDk;x-RjvX-@FkI!a`^q08NwJJ$vMMdz9nsYG*-n+PLxVfCMDUrR)f4 z-c1$5v#r8D!xOVom0cH_RF{!%vZ;wmU^ii|eHAt<>+TE-ER}PxZj~{}pB6;Ov>}!XHsXd@e2BWk6aI#*m%A>m^}MT+^}0!&5|%`>)H=> zEj+&x=CFpdUVE{(H1d|F->AdwYstBw1IQ{FS73E2MZ!A3goFHwl4F{Je0%Iex(pBk zFj!JBM7!{DR{d-M&-cc2TJhB(1<`Xx%+XmmBn4Z@g^s{J$*wYVTBAt(qP~1p7wH2N z+3_z6GfvPQIwf#$*njJV&wlRGd+)vXmugf0@(+LOx4+q{Hx~ZwfBaAX_5a@b<{Pi9 zt;~PsrI-Hl+e^3hj!)0cz4Ol1_y6SIzxK|_TetrF)=&TOFTeW>l*V!`7wNxmqf*dq zOS@FlO^tSM_KwQCI+?981PZAv({bmG=g;YZG97~p@}NH2{Db?*A6Jf^x;@>|W^7tF zY+PS|n1q}=Jc#9*SN-U??wy&~nynN?&3FskX=EjYIu5A5L2~qC%Jv5286y+p%XdJt zZke{es;ZH7z<#J}egm@BaSzd{d24ax%nl?>%lzIY-4(9uI6c^o6GUe-H!}-!n1s;} zJP&OKTGd}}y)pan+=)xRj;QZOH^ZW-P8MHgyoS!vm@{5;2k1bEFghADrMF^SqP6_k zqWt=FX7(h}Znv9fyp;eqqOshg;IoFF9)bQwdIXw{UWg3TaCS(8hA-|T+-dQf$BrEbpFbQk_{%!UhSR1;-iWa7 z1Q>l|zzNpecYFI}SdpwLV?SNxcjqtNs#Mhr+gLGi| zYw5t^dc_HetmhT32_sQx@dpH;5K^!kxEZhO-J~W`=(r;dz$fP}ewM9F(MbHDP{{}a z83KszJ@m`W`HQ;8Zx`EhI(g)<9OU#WPJ6e@n57#aLGA(Be;-~qzLz<@DO7^1IRwz0 zBfu^%mDO%@{=6wAOBLh}^!9wJw8T5BJz*2xO2?}kwsg{+&cM#|W9n7l$5s8D1PQ?R z=$h(=ufc;S-FXQg05tW_8b|DsrYj5^-0zmz63X}tHKGK^XI!HOk~ZqeVxxXqBIS%4 zi9(}x6>AF^s91NB1iU~=ZkGl)CY^>4e)xh(L-4;>6I*>H$f4!LXXC+dObtFo#9>e{P9#omBHJYp4Ey{wl zmqC_my0$(WR1)w4CAqO5aI7T5)6}@BRd#d=w2X3W)&|7Axn|5-Fz~Bnp+o zFp!#%J{SVdh}SHM-8-xmu}5#bD2j$Fx9zOFdgDb%PhIcVH}~lx1-Ajl+;Uz}2WXEjQm_Re>dCx%tg0`Q z-U(P4!?2Zg3$v79Ab9Yp!IDl}2!_cRApyeoD;C);v5KBi!N|Z&EdL z2-mm3!-`{GuAu-)$DMyzY$(r5q@1B3LAS?Z4kv@v+1%H^0}e&*)kO-n0cYJJ>>^KV zDMBx_iPrdCvO$)xKwFPwI~{wZTV^A2&J+?j4IOB(?u-riy}BlA4Nq@mxzzNoT?lU! zGZ_x&s1NCPJ3qfUf<%FdKcJt9SEOJ&VA^`N*7d>{#)i8AO~oJ74~aJbb-dxy%)o&L z*fJr@=3QV@K+-nl72DJc5-Dd>NR&{$EnuK7A5wmaW`GxH=2888Ko=?4130FeR$S87 z0Dru;g53@z9;q_K+foXM2a+cKL^1Jn;^l}(q6G1_fDIB4yny(J^)s3x1$P3DiKkA6 z_<@WAFT=RazJPHcDaJSdc`@TBB~p%YBuX%D3)mpzzzZ06O&xH|cxdY8xjr*#KUZtl z78RS+gI0z6h%Qoa*Gc_bhIsIlQb0VA6!Dw>MltcD5-CSK5{1M+F2G~DNWos<1Yr@+ zz6`qsT{k_v_NrM+WF0(b$ZTsiP-9R@h$jVi04Jw#$Q+e+$=54S^Ew&O1^RV|1At@w z(q)@$S*slARfv>Pt}UWKxj@p&{k61m-7y0q4AQhQ+#Un~MP@^aJLVtL*NnJEo1F>0 zAy2n#M^>NxmEXi_-DPKh^ydQ3t#;=1fS7J^b9!qiq+g6GtNK4Nb%a>vH8)%4^k7Rv zH%@zNdc}bq!O+Q6b&-Ny;MX%sw{u4VzBQQAo8wh|`<#~<2prHalt2o0gJlvEj-)gh zSl}#7w$KfJToIWfxPCpTixk{XrBK>APdop`yLoPLwA~o3YqOc2WY?y* zxTLJkxNT!*7>Ezrv6$bOSPYi{Vq#6!yNLubmOJ|y#pMSzBcqeJJUxxsUbOEtkK2+p z4}Nlm_!J8apiPgWY7d0fBQJZ;6qRj%+yP-bm;KHat$WlxJPwOZhJPprw?40DgS*r0 zT#DaN8{V!%kGM7|xDPm80q?y(&AB)x653Nt6%^yfd_pLUlN9Vjj!I7J$!i^N&7y*O znW^QKj#{Ue4A(mKrC8MMP7%dvqI=D;47cNBk*rb)*^$U>&s$khiK#u$hlHS(q~Jk{ z!N$WI@#?L^Q`MswkzxF>pik-|1rH;Px(0P%A4YJfx%BA%cnO2I>rfst9O5DQ(iwJqW@dc1g@hRrAhb#xMkN{*5F zTIP*K^V-?fu;n;Tl~0>_GBP@K>E~6~{H}=DGuvF$N%mI%N2!bH9o@A}+RSKcw$)tb z38y&PRQ07BI*Z47Sv;XO3^G~}vGs_nk@bAK{x~Q(9b$eIi?IyL7W)T7W=~mu6pMw< zHQV7GhfON>1~d*(k`0q{Mys*RZH9VGmv)kZVaOzYIip)bR<`SpHgR6|T8-{xQ#KHe z6EH^1nG`%geu$e}qo=NFq=Rp|qo%}|vn&ztjI5U<7e>i!$V!d9E5qrrcRsPIm}y6H zK3i=4*)j-=643Eowl7X%hA~ofL6=JU;C6VRYg2d7YM5A0$M_L#QVn0h4tMA0J}<tu$ z4sEUs4_E$&V%rwUZ#IG8naU^kRQA6#bsNCpX_ND*^AqNOx+l_ePj&qHmVf<){|6kY BcJcrK literal 0 HcmV?d00001 diff --git a/src/snapshot/metadata.rs b/src/snapshot/metadata.rs index 79f7a2d0..273a7665 100644 --- a/src/snapshot/metadata.rs +++ b/src/snapshot/metadata.rs @@ -70,6 +70,12 @@ pub struct InstanceMetadata { /// A schema provided via a JSON file, if one exists. Will be `None` for /// all non-JSON middleware. pub schema: Option, + + /// A custom name specified via meta.json or model.json files. If present, + /// this name will be used for the instance while the filesystem name will + /// be slugified to remove illegal characters. + #[serde(skip_serializing_if = "Option::is_none")] + pub specified_name: Option, } impl InstanceMetadata { @@ -82,6 +88,7 @@ impl InstanceMetadata { specified_id: None, middleware: None, schema: None, + specified_name: None, } } @@ -130,6 +137,13 @@ impl InstanceMetadata { pub fn schema(self, schema: Option) -> Self { Self { schema, ..self } } + + pub fn specified_name(self, specified_name: Option) -> Self { + Self { + specified_name, + ..self + } + } } impl Default for InstanceMetadata { diff --git a/src/snapshot_middleware/json_model.rs b/src/snapshot_middleware/json_model.rs index d19792cb..185ab9ee 100644 --- a/src/snapshot_middleware/json_model.rs +++ b/src/snapshot_middleware/json_model.rs @@ -81,6 +81,7 @@ pub fn syncback_json_model<'sync>( // schemas will ever exist in one project for it to matter, but it // could have a performance cost. model.schema = old_inst.metadata().schema.clone(); + model.name = old_inst.metadata().specified_name.clone(); } Ok(SyncbackReturn { diff --git a/src/snapshot_middleware/lua.rs b/src/snapshot_middleware/lua.rs index eaa90952..f0ddea11 100644 --- a/src/snapshot_middleware/lua.rs +++ b/src/snapshot_middleware/lua.rs @@ -158,8 +158,14 @@ pub fn syncback_lua<'sync>( if !meta.is_empty() { let parent_location = snapshot.path.parent_err()?; + let instance_name = &snapshot.new_inst().name; + let meta_name = if crate::syncback::validate_file_name(instance_name).is_err() { + crate::syncback::slugify_name(instance_name) + } else { + instance_name.clone() + }; fs_snapshot.add_file( - parent_location.join(format!("{}.meta.json", new_inst.name)), + parent_location.join(format!("{}.meta.json", meta_name)), serde_json::to_vec_pretty(&meta).context("cannot serialize metadata")?, ); } diff --git a/src/snapshot_middleware/meta_file.rs b/src/snapshot_middleware/meta_file.rs index 84ca9a80..23a18918 100644 --- a/src/snapshot_middleware/meta_file.rs +++ b/src/snapshot_middleware/meta_file.rs @@ -10,7 +10,10 @@ use rbx_dom_weak::{ use serde::{Deserialize, Serialize}; use crate::{ - json, resolution::UnresolvedValue, snapshot::InstanceSnapshot, syncback::SyncbackSnapshot, + json, + resolution::UnresolvedValue, + snapshot::InstanceSnapshot, + syncback::{validate_file_name, SyncbackSnapshot}, RojoRef, }; @@ -36,6 +39,9 @@ pub struct AdjacentMetadata { #[serde(default, skip_serializing_if = "IndexMap::is_empty")] pub attributes: IndexMap, + #[serde(skip_serializing_if = "Option::is_none")] + pub name: Option, + #[serde(skip)] pub path: PathBuf, } @@ -144,6 +150,24 @@ impl AdjacentMetadata { } } + let name = snapshot + .old_inst() + .and_then(|inst| inst.metadata().specified_name.clone()) + .or_else(|| { + // If this is a new instance and its name is invalid for the filesystem, + // we need to specify the name in meta.json so it can be preserved + if snapshot.old_inst().is_none() { + let instance_name = &snapshot.new_inst().name; + if validate_file_name(instance_name).is_err() { + Some(instance_name.clone()) + } else { + None + } + } else { + None + } + }); + Ok(Some(Self { ignore_unknown_instances: if ignore_unknown_instances { Some(true) @@ -155,6 +179,7 @@ impl AdjacentMetadata { path, id: None, schema, + name, })) } @@ -213,11 +238,23 @@ impl AdjacentMetadata { Ok(()) } + fn apply_name(&mut self, snapshot: &mut InstanceSnapshot) -> anyhow::Result<()> { + if self.name.is_some() && snapshot.metadata.specified_name.is_some() { + anyhow::bail!( + "cannot specify a name using {} (instance has a name from somewhere else)", + self.path.display() + ); + } + snapshot.metadata.specified_name = self.name.take(); + Ok(()) + } + pub fn apply_all(&mut self, snapshot: &mut InstanceSnapshot) -> anyhow::Result<()> { self.apply_ignore_unknown_instances(snapshot); self.apply_properties(snapshot)?; self.apply_id(snapshot)?; self.apply_schema(snapshot)?; + self.apply_name(snapshot)?; Ok(()) } @@ -226,11 +263,13 @@ impl AdjacentMetadata { /// /// - The number of properties and attributes is 0 /// - `ignore_unknown_instances` is None + /// - `name` is None #[inline] pub fn is_empty(&self) -> bool { self.attributes.is_empty() && self.properties.is_empty() && self.ignore_unknown_instances.is_none() + && self.name.is_none() } // TODO: Add method to allow selectively applying parts of metadata and @@ -262,6 +301,9 @@ pub struct DirectoryMetadata { #[serde(skip_serializing_if = "Option::is_none")] pub class_name: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub name: Option, + #[serde(skip)] pub path: PathBuf, } @@ -372,6 +414,24 @@ impl DirectoryMetadata { } } + let name = snapshot + .old_inst() + .and_then(|inst| inst.metadata().specified_name.clone()) + .or_else(|| { + // If this is a new instance and its name is invalid for the filesystem, + // we need to specify the name in meta.json so it can be preserved + if snapshot.old_inst().is_none() { + let instance_name = &snapshot.new_inst().name; + if validate_file_name(instance_name).is_err() { + Some(instance_name.clone()) + } else { + None + } + } else { + None + } + }); + Ok(Some(Self { ignore_unknown_instances: if ignore_unknown_instances { Some(true) @@ -384,6 +444,7 @@ impl DirectoryMetadata { path, id: None, schema, + name, })) } @@ -393,6 +454,7 @@ impl DirectoryMetadata { self.apply_properties(snapshot)?; self.apply_id(snapshot)?; self.apply_schema(snapshot)?; + self.apply_name(snapshot)?; Ok(()) } @@ -464,17 +526,30 @@ impl DirectoryMetadata { snapshot.metadata.schema = self.schema.take(); Ok(()) } + + fn apply_name(&mut self, snapshot: &mut InstanceSnapshot) -> anyhow::Result<()> { + if self.name.is_some() && snapshot.metadata.specified_name.is_some() { + anyhow::bail!( + "cannot specify a name using {} (instance has a name from somewhere else)", + self.path.display() + ); + } + snapshot.metadata.specified_name = self.name.take(); + Ok(()) + } /// Returns whether the metadata is 'empty', meaning it doesn't have anything /// worth persisting in it. Specifically: /// /// - The number of properties and attributes is 0 /// - `ignore_unknown_instances` is None /// - `class_name` is either None or not Some("Folder") + /// - `name` is None #[inline] pub fn is_empty(&self) -> bool { self.attributes.is_empty() && self.properties.is_empty() && self.ignore_unknown_instances.is_none() + && self.name.is_none() && if let Some(class) = &self.class_name { class == "Folder" } else { diff --git a/src/syncback/file_names.rs b/src/syncback/file_names.rs index 4653aabd..3369c640 100644 --- a/src/syncback/file_names.rs +++ b/src/syncback/file_names.rs @@ -35,14 +35,23 @@ pub fn name_for_inst<'old>( | Middleware::CsvDir | Middleware::ServerScriptDir | Middleware::ClientScriptDir - | Middleware::ModuleScriptDir => Cow::Owned(new_inst.name.clone()), + | Middleware::ModuleScriptDir => { + let name = if validate_file_name(&new_inst.name).is_err() { + Cow::Owned(slugify_name(&new_inst.name)) + } else { + Cow::Owned(new_inst.name.clone()) + }; + name + } _ => { let extension = extension_for_middleware(middleware); - let name = &new_inst.name; - validate_file_name(name).with_context(|| { - format!("name '{name}' is not legal to write to the file system") - })?; - Cow::Owned(format!("{name}.{extension}")) + let final_name = if validate_file_name(&new_inst.name).is_err() { + slugify_name(&new_inst.name) + } else { + new_inst.name.clone() + }; + + Cow::Owned(format!("{final_name}.{extension}")) } }) } @@ -94,6 +103,39 @@ const INVALID_WINDOWS_NAMES: [&str; 22] = [ /// in a file's name. const FORBIDDEN_CHARS: [char; 9] = ['<', '>', ':', '"', '/', '|', '?', '*', '\\']; +/// Slugifies a name by replacing forbidden characters with underscores +/// and ensuring the result is a valid file name +pub fn slugify_name(name: &str) -> String { + let mut result = String::with_capacity(name.len()); + + for ch in name.chars() { + if FORBIDDEN_CHARS.contains(&ch) { + result.push('_'); + } else { + result.push(ch); + } + } + + // Handle Windows reserved names by appending an underscore + let result_lower = result.to_lowercase(); + for forbidden in INVALID_WINDOWS_NAMES { + if result_lower == forbidden.to_lowercase() { + result.push('_'); + break; + } + } + + while result.ends_with(' ') || result.ends_with('.') { + result.pop(); + } + + if result.is_empty() || result.chars().all(|c| c == '_') { + result = "instance".to_string(); + } + + result +} + /// Validates a provided file name to ensure it's allowed on the file system. An /// error is returned if the name isn't allowed, indicating why. /// This takes into account rules for Windows, MacOS, and Linux. diff --git a/src/syncback/mod.rs b/src/syncback/mod.rs index 61d26674..57d86f24 100644 --- a/src/syncback/mod.rs +++ b/src/syncback/mod.rs @@ -28,7 +28,7 @@ use crate::{ Project, }; -pub use file_names::{extension_for_middleware, name_for_inst, validate_file_name}; +pub use file_names::{extension_for_middleware, name_for_inst, slugify_name, validate_file_name}; pub use fs_snapshot::FsSnapshot; pub use hash::*; pub use property_filter::{filter_properties, filter_properties_preallocated}; @@ -360,6 +360,19 @@ pub fn get_best_middleware(snapshot: &SyncbackSnapshot) -> Middleware { } } + // If the instance name is invalid for the filesystem, use directory middleware + // to allow preserving the name in meta.json + if crate::syncback::file_names::validate_file_name(&inst.name).is_err() { + middleware = match middleware { + Middleware::ServerScript => Middleware::ServerScriptDir, + Middleware::ClientScript => Middleware::ClientScriptDir, + Middleware::ModuleScript => Middleware::ModuleScriptDir, + Middleware::Csv => Middleware::CsvDir, + Middleware::JsonModel | Middleware::Text => Middleware::Dir, + _ => middleware, + } + } + if middleware == Middleware::Rbxm { middleware = match env::var(DEBUG_MODEL_FORMAT_VAR) { Ok(value) if value == "1" => Middleware::Rbxmx, diff --git a/tests/tests/syncback.rs b/tests/tests/syncback.rs index 31553496..c21824ce 100644 --- a/tests/tests/syncback.rs +++ b/tests/tests/syncback.rs @@ -86,4 +86,7 @@ syncback_tests! { sync_rules => ["src/module.modulescript", "src/text.text"], // Ensures that the `syncUnscriptable` setting works unscriptable_properties => ["default.project.json"], + // Ensures that instances with names containing illegal characters get slugified filenames + // and preserve their original names in meta.json + slugified_name => ["src/_Script/init.meta.json", "src/_Script/init.server.luau", "src/_Folder/init.meta.json"], } From 5c396322d9290bd4ce26a245eb2f0df80d6aca73 Mon Sep 17 00:00:00 2001 From: astrid Date: Mon, 15 Dec 2025 19:08:18 +0100 Subject: [PATCH 02/15] fix: `name` prop not properly syncing --- ...ests__build__slugified_name_roundtrip.snap | 20 +++++++++++++++++++ .../_Script/init.meta.json | 4 ++++ .../_Script/init.server.luau | 2 ++ .../default.project.json | 6 ++++++ .../src/_Script/init.meta.json | 3 +++ .../src/_Script/init.server.luau | 1 + src/snapshot_middleware/meta_file.rs | 6 ++++++ tests/tests/build.rs | 1 + 8 files changed, 43 insertions(+) create mode 100644 rojo-test/build-test-snapshots/end_to_end__tests__build__slugified_name_roundtrip.snap create mode 100644 rojo-test/build-tests/slugified_name_roundtrip/_Script/init.meta.json create mode 100644 rojo-test/build-tests/slugified_name_roundtrip/_Script/init.server.luau create mode 100644 rojo-test/build-tests/slugified_name_roundtrip/default.project.json create mode 100644 rojo-test/build-tests/slugified_name_roundtrip/src/_Script/init.meta.json create mode 100644 rojo-test/build-tests/slugified_name_roundtrip/src/_Script/init.server.luau diff --git a/rojo-test/build-test-snapshots/end_to_end__tests__build__slugified_name_roundtrip.snap b/rojo-test/build-test-snapshots/end_to_end__tests__build__slugified_name_roundtrip.snap new file mode 100644 index 00000000..c88e9a80 --- /dev/null +++ b/rojo-test/build-test-snapshots/end_to_end__tests__build__slugified_name_roundtrip.snap @@ -0,0 +1,20 @@ +--- +source: tests/tests/build.rs +assertion_line: 108 +expression: contents +--- + + + + slugified_name_roundtrip + + + + /Script + 0 + + + + + diff --git a/rojo-test/build-tests/slugified_name_roundtrip/_Script/init.meta.json b/rojo-test/build-tests/slugified_name_roundtrip/_Script/init.meta.json new file mode 100644 index 00000000..eb7e860c --- /dev/null +++ b/rojo-test/build-tests/slugified_name_roundtrip/_Script/init.meta.json @@ -0,0 +1,4 @@ +{ + "name": "/Script" +} + diff --git a/rojo-test/build-tests/slugified_name_roundtrip/_Script/init.server.luau b/rojo-test/build-tests/slugified_name_roundtrip/_Script/init.server.luau new file mode 100644 index 00000000..b29a52f4 --- /dev/null +++ b/rojo-test/build-tests/slugified_name_roundtrip/_Script/init.server.luau @@ -0,0 +1,2 @@ +print("Hello world!") + diff --git a/rojo-test/build-tests/slugified_name_roundtrip/default.project.json b/rojo-test/build-tests/slugified_name_roundtrip/default.project.json new file mode 100644 index 00000000..79f6f8a5 --- /dev/null +++ b/rojo-test/build-tests/slugified_name_roundtrip/default.project.json @@ -0,0 +1,6 @@ +{ + "name": "slugified_name_roundtrip", + "tree": { + "$path": "src" + } +} diff --git a/rojo-test/build-tests/slugified_name_roundtrip/src/_Script/init.meta.json b/rojo-test/build-tests/slugified_name_roundtrip/src/_Script/init.meta.json new file mode 100644 index 00000000..617a69b2 --- /dev/null +++ b/rojo-test/build-tests/slugified_name_roundtrip/src/_Script/init.meta.json @@ -0,0 +1,3 @@ +{ + "name": "/Script" +} diff --git a/rojo-test/build-tests/slugified_name_roundtrip/src/_Script/init.server.luau b/rojo-test/build-tests/slugified_name_roundtrip/src/_Script/init.server.luau new file mode 100644 index 00000000..658f93e6 --- /dev/null +++ b/rojo-test/build-tests/slugified_name_roundtrip/src/_Script/init.server.luau @@ -0,0 +1 @@ +print("Hello world!") diff --git a/src/snapshot_middleware/meta_file.rs b/src/snapshot_middleware/meta_file.rs index 23a18918..3a6a6fda 100644 --- a/src/snapshot_middleware/meta_file.rs +++ b/src/snapshot_middleware/meta_file.rs @@ -245,6 +245,9 @@ impl AdjacentMetadata { self.path.display() ); } + if let Some(name) = &self.name { + snapshot.name = name.clone().into(); + } snapshot.metadata.specified_name = self.name.take(); Ok(()) } @@ -534,6 +537,9 @@ impl DirectoryMetadata { self.path.display() ); } + if let Some(name) = &self.name { + snapshot.name = name.clone().into(); + } snapshot.metadata.specified_name = self.name.take(); Ok(()) } diff --git a/tests/tests/build.rs b/tests/tests/build.rs index 8c533115..db3cdcc8 100644 --- a/tests/tests/build.rs +++ b/tests/tests/build.rs @@ -55,6 +55,7 @@ gen_build_tests! { script_meta_disabled, server_in_folder, server_init, + slugified_name_roundtrip, txt, txt_in_folder, unresolved_values, From 790312a5b03aef3cc588e18d913b57e2887896a5 Mon Sep 17 00:00:00 2001 From: astrid Date: Mon, 15 Dec 2025 20:26:25 +0100 Subject: [PATCH 03/15] fix: lack of .model.json support --- ...__tests__build__model_json_name_input.snap | 23 ++++++++++++++++++ .../default.project.json | 11 +++++++++ .../model_json_name_input/src/Bar.model.json | 5 ++++ ...syncback_util__model_json_name-stdout.snap | 6 +++++ ...__model_json_name-src__foo.model.json.snap | 9 +++++++ .../input-project/default.project.json | 11 +++++++++ .../input-project/src/foo.model.json | 5 ++++ .../syncback-tests/model_json_name/input.rbxl | Bin 0 -> 310 bytes src/snapshot_middleware/json_model.rs | 20 ++++++--------- tests/tests/build.rs | 1 + tests/tests/syncback.rs | 2 ++ 11 files changed, 80 insertions(+), 13 deletions(-) create mode 100644 rojo-test/build-test-snapshots/end_to_end__tests__build__model_json_name_input.snap create mode 100644 rojo-test/build-tests/model_json_name_input/default.project.json create mode 100644 rojo-test/build-tests/model_json_name_input/src/Bar.model.json create mode 100644 rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__model_json_name-stdout.snap create mode 100644 rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__model_json_name-src__foo.model.json.snap create mode 100644 rojo-test/syncback-tests/model_json_name/input-project/default.project.json create mode 100644 rojo-test/syncback-tests/model_json_name/input-project/src/foo.model.json create mode 100644 rojo-test/syncback-tests/model_json_name/input.rbxl diff --git a/rojo-test/build-test-snapshots/end_to_end__tests__build__model_json_name_input.snap b/rojo-test/build-test-snapshots/end_to_end__tests__build__model_json_name_input.snap new file mode 100644 index 00000000..018fcd34 --- /dev/null +++ b/rojo-test/build-test-snapshots/end_to_end__tests__build__model_json_name_input.snap @@ -0,0 +1,23 @@ +--- +source: tests/tests/build.rs +assertion_line: 109 +expression: contents +--- + + + + model_json_name_input + + + + Workspace + false + + + + /Bar + + + + + diff --git a/rojo-test/build-tests/model_json_name_input/default.project.json b/rojo-test/build-tests/model_json_name_input/default.project.json new file mode 100644 index 00000000..6c01ae9e --- /dev/null +++ b/rojo-test/build-tests/model_json_name_input/default.project.json @@ -0,0 +1,11 @@ +{ + "name": "model_json_name_input", + "tree": { + "$className": "DataModel", + "Workspace": { + "$className": "Workspace", + "$path": "src" + } + } +} + diff --git a/rojo-test/build-tests/model_json_name_input/src/Bar.model.json b/rojo-test/build-tests/model_json_name_input/src/Bar.model.json new file mode 100644 index 00000000..4fef832e --- /dev/null +++ b/rojo-test/build-tests/model_json_name_input/src/Bar.model.json @@ -0,0 +1,5 @@ +{ + "name": "/Bar", + "className": "StringValue" +} + diff --git a/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__model_json_name-stdout.snap b/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__model_json_name-stdout.snap new file mode 100644 index 00000000..c8e32ca7 --- /dev/null +++ b/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__model_json_name-stdout.snap @@ -0,0 +1,6 @@ +--- +source: tests/rojo_test/syncback_util.rs +assertion_line: 101 +expression: "String::from_utf8_lossy(&output.stdout)" +--- + diff --git a/rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__model_json_name-src__foo.model.json.snap b/rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__model_json_name-src__foo.model.json.snap new file mode 100644 index 00000000..ad4017dd --- /dev/null +++ b/rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__model_json_name-src__foo.model.json.snap @@ -0,0 +1,9 @@ +--- +source: tests/tests/syncback.rs +assertion_line: 31 +expression: src/foo.model.json +--- +{ + "name": "/Bar", + "className": "StringValue" +} diff --git a/rojo-test/syncback-tests/model_json_name/input-project/default.project.json b/rojo-test/syncback-tests/model_json_name/input-project/default.project.json new file mode 100644 index 00000000..8eaa84a8 --- /dev/null +++ b/rojo-test/syncback-tests/model_json_name/input-project/default.project.json @@ -0,0 +1,11 @@ +{ + "name": "model_json_name", + "tree": { + "$className": "DataModel", + "Workspace": { + "$className": "Workspace", + "$path": "src" + } + } +} + diff --git a/rojo-test/syncback-tests/model_json_name/input-project/src/foo.model.json b/rojo-test/syncback-tests/model_json_name/input-project/src/foo.model.json new file mode 100644 index 00000000..4fef832e --- /dev/null +++ b/rojo-test/syncback-tests/model_json_name/input-project/src/foo.model.json @@ -0,0 +1,5 @@ +{ + "name": "/Bar", + "className": "StringValue" +} + diff --git a/rojo-test/syncback-tests/model_json_name/input.rbxl b/rojo-test/syncback-tests/model_json_name/input.rbxl new file mode 100644 index 0000000000000000000000000000000000000000..9c8f15100c385367747f981e911c47875d473822 GIT binary patch literal 310 zcmcC1%1_G4uTbp#&&wsn#lXPC0Kq^C3_SgUL*#%g87Th)FNnbn#K9#+nR)49i8-aI z42)1&kZM^VTN=v$zyp%u1mf`gqU_>=#N<>)s9Gk*fFS<>aiEkaOfegX!2-m7iMgqa zAcnqEVi8Cg$aX0p27wRUAQRxqPz(pDl?Td#+yDX}_&{off!HrKHKjNpvn;>FH#5B` uu_QA;kBK25$S*_yC<$|cDbV|j3==?M00d0Tu6{0H7RU||u+fKvmK^{=_%Bre literal 0 HcmV?d00001 diff --git a/src/snapshot_middleware/json_model.rs b/src/snapshot_middleware/json_model.rs index 185ab9ee..e7b929b5 100644 --- a/src/snapshot_middleware/json_model.rs +++ b/src/snapshot_middleware/json_model.rs @@ -35,19 +35,12 @@ pub fn snapshot_json_model( format!("File is not a valid JSON model: {}", path.display()) })?; - if let Some(top_level_name) = &instance.name { - let new_name = format!("{}.model.json", top_level_name); + // If the JSON has a name property, preserve it in metadata for syncback + let specified_name = instance.name.clone(); - log::warn!( - "Model at path {} had a top-level Name field. \ - This field has been ignored since Rojo 6.0.\n\ - Consider removing this field and renaming the file to {}.", - new_name, - path.display() - ); - } - - instance.name = Some(name.to_owned()); + // Use the name from JSON if present, otherwise fall back to filename-derived name + let instance_name = specified_name.clone().unwrap_or_else(|| name.to_owned()); + instance.name = Some(instance_name); let id = instance.id.take().map(RojoRef::new); let schema = instance.schema.take(); @@ -62,7 +55,8 @@ pub fn snapshot_json_model( .relevant_paths(vec![path.to_path_buf()]) .context(context) .specified_id(id) - .schema(schema); + .schema(schema) + .specified_name(specified_name); Ok(Some(snapshot)) } diff --git a/tests/tests/build.rs b/tests/tests/build.rs index db3cdcc8..8776d7ef 100644 --- a/tests/tests/build.rs +++ b/tests/tests/build.rs @@ -56,6 +56,7 @@ gen_build_tests! { server_in_folder, server_init, slugified_name_roundtrip, + model_json_name_input, txt, txt_in_folder, unresolved_values, diff --git a/tests/tests/syncback.rs b/tests/tests/syncback.rs index c21824ce..e2b1f6f5 100644 --- a/tests/tests/syncback.rs +++ b/tests/tests/syncback.rs @@ -89,4 +89,6 @@ syncback_tests! { // Ensures that instances with names containing illegal characters get slugified filenames // and preserve their original names in meta.json slugified_name => ["src/_Script/init.meta.json", "src/_Script/init.server.luau", "src/_Folder/init.meta.json"], + // Ensures that .model.json files preserve the name property + model_json_name => ["src/foo.model.json"], } From 73dab330b57969b4e11442ee8a834bc18eb9702f Mon Sep 17 00:00:00 2001 From: astrid Date: Mon, 15 Dec 2025 20:32:28 +0100 Subject: [PATCH 04/15] test: remove oudated json_model_legacy_name test --- ...nd__tests__build__json_model_legacy_name.snap | 16 ---------------- .../json_model_legacy_name/default.project.json | 6 ------ .../folder/Expected Name.model.json | 4 ---- tests/tests/build.rs | 1 - 4 files changed, 27 deletions(-) delete mode 100644 rojo-test/build-test-snapshots/end_to_end__tests__build__json_model_legacy_name.snap delete mode 100644 rojo-test/build-tests/json_model_legacy_name/default.project.json delete mode 100644 rojo-test/build-tests/json_model_legacy_name/folder/Expected Name.model.json diff --git a/rojo-test/build-test-snapshots/end_to_end__tests__build__json_model_legacy_name.snap b/rojo-test/build-test-snapshots/end_to_end__tests__build__json_model_legacy_name.snap deleted file mode 100644 index ba8299cf..00000000 --- a/rojo-test/build-test-snapshots/end_to_end__tests__build__json_model_legacy_name.snap +++ /dev/null @@ -1,16 +0,0 @@ ---- -source: tests/tests/build.rs -expression: contents ---- - - - - json_model_legacy_name - - - - Expected Name - - - - diff --git a/rojo-test/build-tests/json_model_legacy_name/default.project.json b/rojo-test/build-tests/json_model_legacy_name/default.project.json deleted file mode 100644 index c7c56c0d..00000000 --- a/rojo-test/build-tests/json_model_legacy_name/default.project.json +++ /dev/null @@ -1,6 +0,0 @@ -{ - "name": "json_model_legacy_name", - "tree": { - "$path": "folder" - } -} \ No newline at end of file diff --git a/rojo-test/build-tests/json_model_legacy_name/folder/Expected Name.model.json b/rojo-test/build-tests/json_model_legacy_name/folder/Expected Name.model.json deleted file mode 100644 index b816d601..00000000 --- a/rojo-test/build-tests/json_model_legacy_name/folder/Expected Name.model.json +++ /dev/null @@ -1,4 +0,0 @@ -{ - "Name": "Overridden Name", - "ClassName": "Folder" -} \ No newline at end of file diff --git a/tests/tests/build.rs b/tests/tests/build.rs index 8776d7ef..8c3c994a 100644 --- a/tests/tests/build.rs +++ b/tests/tests/build.rs @@ -41,7 +41,6 @@ gen_build_tests! { issue_546, json_as_lua, json_model_in_folder, - json_model_legacy_name, module_in_folder, module_init, nested_runcontext, From 60d150f4c600ff5edf81ee9a2488735e7d62762b Mon Sep 17 00:00:00 2001 From: astrid Date: Thu, 18 Dec 2025 04:43:10 +0100 Subject: [PATCH 05/15] feat: optimize name handling for leaf scripts with invalid names Prefer slugified filenames + adjacent meta files for scripts without children instead of forcing directory creation --- ..._syncback_util__slugified_name-stdout.snap | 5 ++-- src/syncback/mod.rs | 25 +++++++++++++------ tests/tests/syncback.rs | 4 +-- 3 files changed, 21 insertions(+), 13 deletions(-) diff --git a/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__slugified_name-stdout.snap b/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__slugified_name-stdout.snap index b50c2448..13188a47 100644 --- a/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__slugified_name-stdout.snap +++ b/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__slugified_name-stdout.snap @@ -7,8 +7,7 @@ Writing default.project.json Writing src/Camera.rbxm Writing src/Terrain.rbxm Writing src/_Folder/init.meta.json -Writing src/_Script/init.meta.json -Writing src/_Script/init.server.luau +Writing src/_Script.meta.json +Writing src/_Script.server.luau Writing src Writing src/_Folder -Writing src/_Script diff --git a/src/syncback/mod.rs b/src/syncback/mod.rs index 57d86f24..8e0d7d03 100644 --- a/src/syncback/mod.rs +++ b/src/syncback/mod.rs @@ -360,16 +360,25 @@ pub fn get_best_middleware(snapshot: &SyncbackSnapshot) -> Middleware { } } - // If the instance name is invalid for the filesystem, use directory middleware - // to allow preserving the name in meta.json + // If the name is invalid but the instance has no descendants and isn't a + // folder/config/tool, prefer slugified files over creating a directory. + // Only promote to directory when there are children (or dir-like classes). if crate::syncback::file_names::validate_file_name(&inst.name).is_err() { middleware = match middleware { - Middleware::ServerScript => Middleware::ServerScriptDir, - Middleware::ClientScript => Middleware::ClientScriptDir, - Middleware::ModuleScript => Middleware::ModuleScriptDir, - Middleware::Csv => Middleware::CsvDir, - Middleware::JsonModel | Middleware::Text => Middleware::Dir, - _ => middleware, + Middleware::ServerScript | Middleware::ClientScript | Middleware::ModuleScript + if inst.children().is_empty() => + { + middleware + } + Middleware::JsonModel | Middleware::Text if inst.children().is_empty() => middleware, + _ => match middleware { + Middleware::ServerScript => Middleware::ServerScriptDir, + Middleware::ClientScript => Middleware::ClientScriptDir, + Middleware::ModuleScript => Middleware::ModuleScriptDir, + Middleware::Csv => Middleware::CsvDir, + Middleware::JsonModel | Middleware::Text => Middleware::Dir, + _ => middleware, + }, } } diff --git a/tests/tests/syncback.rs b/tests/tests/syncback.rs index e2b1f6f5..10be92a8 100644 --- a/tests/tests/syncback.rs +++ b/tests/tests/syncback.rs @@ -87,8 +87,8 @@ syncback_tests! { // Ensures that the `syncUnscriptable` setting works unscriptable_properties => ["default.project.json"], // Ensures that instances with names containing illegal characters get slugified filenames - // and preserve their original names in meta.json - slugified_name => ["src/_Script/init.meta.json", "src/_Script/init.server.luau", "src/_Folder/init.meta.json"], + // and preserve their original names in meta.json without forcing directories for leaf scripts + slugified_name => ["src/_Script.meta.json", "src/_Script.server.luau", "src/_Folder/init.meta.json"], // Ensures that .model.json files preserve the name property model_json_name => ["src/foo.model.json"], } From 020d72faefa54755236bc076d935d3d34ccff60e Mon Sep 17 00:00:00 2001 From: astrid Date: Thu, 18 Dec 2025 05:10:33 +0100 Subject: [PATCH 06/15] fix: improve middleware selection for actor and other container classes --- src/syncback/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/syncback/mod.rs b/src/syncback/mod.rs index 8e0d7d03..a023fdb9 100644 --- a/src/syncback/mod.rs +++ b/src/syncback/mod.rs @@ -301,6 +301,7 @@ pub fn get_best_middleware(snapshot: &SyncbackSnapshot) -> Middleware { static JSON_MODEL_CLASSES: OnceLock> = OnceLock::new(); let json_model_classes = JSON_MODEL_CLASSES.get_or_init(|| { [ + "Actor", "Sound", "SoundGroup", "Sky", @@ -333,8 +334,6 @@ pub fn get_best_middleware(snapshot: &SyncbackSnapshot) -> Middleware { return override_middleware; } else if let Some(old_middleware) = old_middleware { return old_middleware; - } else if json_model_classes.contains(inst.class.as_str()) { - middleware = Middleware::JsonModel; } else { middleware = match inst.class.as_str() { "Folder" | "Configuration" | "Tool" => Middleware::Dir, @@ -345,6 +344,7 @@ pub fn get_best_middleware(snapshot: &SyncbackSnapshot) -> Middleware { "LocalizationTable" => Middleware::Csv, // This isn't the ideal way to handle this but it works. name if name.ends_with("Value") => Middleware::JsonModel, + _ if json_model_classes.contains(inst.class.as_str()) => Middleware::JsonModel, _ => Middleware::Rbxm, } } From 9a485d88ce4ca4d570dd2178319394b65f45492d Mon Sep 17 00:00:00 2001 From: ari Date: Mon, 12 Jan 2026 14:35:06 +0100 Subject: [PATCH 07/15] Avoid clone in src/snapshot_middleware/lua.rs Co-authored-by: krakow10 --- src/snapshot_middleware/lua.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/snapshot_middleware/lua.rs b/src/snapshot_middleware/lua.rs index f0ddea11..d738e122 100644 --- a/src/snapshot_middleware/lua.rs +++ b/src/snapshot_middleware/lua.rs @@ -159,10 +159,12 @@ pub fn syncback_lua<'sync>( if !meta.is_empty() { let parent_location = snapshot.path.parent_err()?; let instance_name = &snapshot.new_inst().name; + let slugified; let meta_name = if crate::syncback::validate_file_name(instance_name).is_err() { - crate::syncback::slugify_name(instance_name) + slugified = crate::syncback::slugify_name(instance_name); + &slugified } else { - instance_name.clone() + instance_name }; fs_snapshot.add_file( parent_location.join(format!("{}.meta.json", meta_name)), From d13d229eef3aa3eb1f1de1a1cb9adc6612b5070e Mon Sep 17 00:00:00 2001 From: ari Date: Mon, 12 Jan 2026 14:35:18 +0100 Subject: [PATCH 08/15] Avoid clone in src/snapshot_middleware/json_model.rs Co-authored-by: krakow10 --- src/snapshot_middleware/json_model.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/snapshot_middleware/json_model.rs b/src/snapshot_middleware/json_model.rs index e7b929b5..122adf6a 100644 --- a/src/snapshot_middleware/json_model.rs +++ b/src/snapshot_middleware/json_model.rs @@ -39,8 +39,9 @@ pub fn snapshot_json_model( let specified_name = instance.name.clone(); // Use the name from JSON if present, otherwise fall back to filename-derived name - let instance_name = specified_name.clone().unwrap_or_else(|| name.to_owned()); - instance.name = Some(instance_name); + if instance.name.is_none() { + instance.name = Some(name.to_owned()); + } let id = instance.id.take().map(RojoRef::new); let schema = instance.schema.take(); From 3a6aae65f7e7b4ed765a03f7896c4bef5396aa74 Mon Sep 17 00:00:00 2001 From: ari Date: Mon, 12 Jan 2026 14:35:46 +0100 Subject: [PATCH 09/15] Avoid clone in src/syncback/file_names.rs Co-authored-by: krakow10 --- src/syncback/file_names.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/syncback/file_names.rs b/src/syncback/file_names.rs index 3369c640..224926dd 100644 --- a/src/syncback/file_names.rs +++ b/src/syncback/file_names.rs @@ -45,10 +45,12 @@ pub fn name_for_inst<'old>( } _ => { let extension = extension_for_middleware(middleware); + let slugified; let final_name = if validate_file_name(&new_inst.name).is_err() { - slugify_name(&new_inst.name) + slugified = slugify_name(&new_inst.name); + &slugified } else { - new_inst.name.clone() + &new_inst.name }; Cow::Owned(format!("{final_name}.{extension}")) From 0e1364945f7335ecc29796d05f38935e2c00600e Mon Sep 17 00:00:00 2001 From: Astrid Date: Mon, 12 Jan 2026 14:41:12 +0100 Subject: [PATCH 10/15] Avoid clone in src/syncback/file_names.rs --- src/syncback/file_names.rs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/syncback/file_names.rs b/src/syncback/file_names.rs index 224926dd..ff867a83 100644 --- a/src/syncback/file_names.rs +++ b/src/syncback/file_names.rs @@ -8,11 +8,11 @@ use rbx_dom_weak::Instance; use crate::{snapshot::InstanceWithMeta, snapshot_middleware::Middleware}; -pub fn name_for_inst<'old>( +pub fn name_for_inst<'a>( middleware: Middleware, - new_inst: &Instance, - old_inst: Option>, -) -> anyhow::Result> { + new_inst: &'a Instance, + old_inst: Option>, +) -> anyhow::Result> { if let Some(old_inst) = old_inst { if let Some(source) = old_inst.metadata().relevant_paths.first() { source @@ -36,12 +36,11 @@ pub fn name_for_inst<'old>( | Middleware::ServerScriptDir | Middleware::ClientScriptDir | Middleware::ModuleScriptDir => { - let name = if validate_file_name(&new_inst.name).is_err() { + if validate_file_name(&new_inst.name).is_err() { Cow::Owned(slugify_name(&new_inst.name)) } else { - Cow::Owned(new_inst.name.clone()) - }; - name + Cow::Borrowed(&new_inst.name) + } } _ => { let extension = extension_for_middleware(middleware); From 3500ebe02a67231ddb29262edfbd1fc912890fe8 Mon Sep 17 00:00:00 2001 From: astrid Date: Tue, 20 Jan 2026 00:54:18 +0100 Subject: [PATCH 11/15] Update CHANGELOG.md --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7843cb44..7c876312 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,8 +31,10 @@ Making a new release? Simply add the new header with the version and date undern ## Unreleased * Fixed a bug caused by having reference properties (such as `ObjectValue.Value`) that point to an Instance not included in syncback. ([#1179]) +* Implemented support for the "name" property in meta/model JSON files. ([#1187]) [#1179]: https://github.com/rojo-rbx/rojo/pull/1179 +[#1187]: https://github.com/rojo-rbx/rojo/pull/1187 ## [7.7.0-rc.1] (November 27th, 2025) From 791ccfcfd1df75145385519ed0b4d9281f4791dd Mon Sep 17 00:00:00 2001 From: astrid Date: Tue, 20 Jan 2026 00:55:03 +0100 Subject: [PATCH 12/15] Remove addition of 'Actor' to json_model_classes --- src/syncback/mod.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/syncback/mod.rs b/src/syncback/mod.rs index a023fdb9..86ee0751 100644 --- a/src/syncback/mod.rs +++ b/src/syncback/mod.rs @@ -301,7 +301,6 @@ pub fn get_best_middleware(snapshot: &SyncbackSnapshot) -> Middleware { static JSON_MODEL_CLASSES: OnceLock> = OnceLock::new(); let json_model_classes = JSON_MODEL_CLASSES.get_or_init(|| { [ - "Actor", "Sound", "SoundGroup", "Sky", From 78916c8a634d3950d5a975f5e85a839f7661a605 Mon Sep 17 00:00:00 2001 From: astrid Date: Tue, 20 Jan 2026 00:59:34 +0100 Subject: [PATCH 13/15] Revert 2 semantic changes --- src/syncback/mod.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/syncback/mod.rs b/src/syncback/mod.rs index 86ee0751..8e0d7d03 100644 --- a/src/syncback/mod.rs +++ b/src/syncback/mod.rs @@ -333,6 +333,8 @@ pub fn get_best_middleware(snapshot: &SyncbackSnapshot) -> Middleware { return override_middleware; } else if let Some(old_middleware) = old_middleware { return old_middleware; + } else if json_model_classes.contains(inst.class.as_str()) { + middleware = Middleware::JsonModel; } else { middleware = match inst.class.as_str() { "Folder" | "Configuration" | "Tool" => Middleware::Dir, @@ -343,7 +345,6 @@ pub fn get_best_middleware(snapshot: &SyncbackSnapshot) -> Middleware { "LocalizationTable" => Middleware::Csv, // This isn't the ideal way to handle this but it works. name if name.ends_with("Value") => Middleware::JsonModel, - _ if json_model_classes.contains(inst.class.as_str()) => Middleware::JsonModel, _ => Middleware::Rbxm, } } From 5957368c0451fd1e953e0bff50508a8f70b9aad7 Mon Sep 17 00:00:00 2001 From: astrid Date: Tue, 20 Jan 2026 01:08:14 +0100 Subject: [PATCH 14/15] Remove redundant code Can't remember why I added this one --- src/syncback/mod.rs | 22 ---------------------- 1 file changed, 22 deletions(-) diff --git a/src/syncback/mod.rs b/src/syncback/mod.rs index 8e0d7d03..9bc60fe5 100644 --- a/src/syncback/mod.rs +++ b/src/syncback/mod.rs @@ -360,28 +360,6 @@ pub fn get_best_middleware(snapshot: &SyncbackSnapshot) -> Middleware { } } - // If the name is invalid but the instance has no descendants and isn't a - // folder/config/tool, prefer slugified files over creating a directory. - // Only promote to directory when there are children (or dir-like classes). - if crate::syncback::file_names::validate_file_name(&inst.name).is_err() { - middleware = match middleware { - Middleware::ServerScript | Middleware::ClientScript | Middleware::ModuleScript - if inst.children().is_empty() => - { - middleware - } - Middleware::JsonModel | Middleware::Text if inst.children().is_empty() => middleware, - _ => match middleware { - Middleware::ServerScript => Middleware::ServerScriptDir, - Middleware::ClientScript => Middleware::ClientScriptDir, - Middleware::ModuleScript => Middleware::ModuleScriptDir, - Middleware::Csv => Middleware::CsvDir, - Middleware::JsonModel | Middleware::Text => Middleware::Dir, - _ => middleware, - }, - } - } - if middleware == Middleware::Rbxm { middleware = match env::var(DEBUG_MODEL_FORMAT_VAR) { Ok(value) if value == "1" => Middleware::Rbxmx, From 95fe993de3c37c3f9287f5efe3bfcfee286985c0 Mon Sep 17 00:00:00 2001 From: astrid Date: Tue, 24 Feb 2026 01:05:31 +0100 Subject: [PATCH 15/15] feat: auto-resolve init-name conflicts during syncback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a child instance has a Roblox name that would produce a filesystem name of "init" (case-insensitive), syncback now automatically prefixes it with '_' (e.g. "Init" → "_Init.luau") instead of erroring. The corresponding meta.json writes the original name via the `name` property so Rojo can restore it on the next snapshot. The sibling dedup check is updated to use actual on-disk names for existing children and the resolved (init-prefixed) name for new ones, so genuine collisions still error while false positives from the `name` property are avoided. Co-Authored-By: Claude Sonnet 4.6 --- src/snapshot_middleware/dir.rs | 282 +++++++++++++++++++++++++-- src/snapshot_middleware/meta_file.rs | 25 ++- src/syncback/file_names.rs | 20 +- src/syncback/snapshot.rs | 18 ++ 4 files changed, 323 insertions(+), 22 deletions(-) diff --git a/src/snapshot_middleware/dir.rs b/src/snapshot_middleware/dir.rs index fb444c37..50e211fd 100644 --- a/src/snapshot_middleware/dir.rs +++ b/src/snapshot_middleware/dir.rs @@ -8,7 +8,7 @@ use memofs::{DirEntry, Vfs}; use crate::{ snapshot::{InstanceContext, InstanceMetadata, InstanceSnapshot, InstigatingSource}, - syncback::{hash_instance, FsSnapshot, SyncbackReturn, SyncbackSnapshot}, + syncback::{hash_instance, slugify_name, FsSnapshot, SyncbackReturn, SyncbackSnapshot}, }; use super::{meta_file::DirectoryMetadata, snapshot_from_vfs}; @@ -134,12 +134,39 @@ pub fn syncback_dir_no_meta<'sync>( let mut children = Vec::new(); let mut removed_children = Vec::new(); - // We have to enforce unique child names for the file system. - let mut child_names = HashSet::with_capacity(new_inst.children().len()); + // Build the old child map early so it can be used for deduplication below. + let mut old_child_map = HashMap::new(); + if let Some(old_inst) = snapshot.old_inst() { + for child in old_inst.children() { + let inst = snapshot.get_old_instance(*child).unwrap(); + old_child_map.insert(inst.name(), inst); + } + } + + // Enforce unique filesystem names. Uses actual on-disk names for existing + // children and resolved names (with init-prefix) for new ones. + let mut fs_child_names = HashSet::with_capacity(new_inst.children().len()); let mut duplicate_set = HashSet::new(); for child_ref in new_inst.children() { let child = snapshot.get_new_instance(*child_ref).unwrap(); - if !child_names.insert(child.name.to_lowercase()) { + let fs_name = old_child_map + .get(child.name.as_str()) + .and_then(|old| old.metadata().relevant_paths.first()) + .and_then(|p| p.file_name()) + .and_then(|n| n.to_str()) + .map(|s| s.to_lowercase()) + .unwrap_or_else(|| { + let slug = slugify_name(&child.name); + let slug_lower = slug.to_lowercase(); + // Mirror name_for_inst's init-prefix. + if slug_lower == "init" { + format!("_{slug_lower}") + } else { + slug_lower + } + }); + + if !fs_child_names.insert(fs_name) { duplicate_set.insert(child.name.as_str()); } } @@ -153,13 +180,7 @@ pub fn syncback_dir_no_meta<'sync>( anyhow::bail!("Instance has more than 25 children with duplicate names"); } - if let Some(old_inst) = snapshot.old_inst() { - let mut old_child_map = HashMap::with_capacity(old_inst.children().len()); - for child in old_inst.children() { - let inst = snapshot.get_old_instance(*child).unwrap(); - old_child_map.insert(inst.name(), inst); - } - + if snapshot.old_inst().is_some() { for new_child_ref in new_inst.children() { let new_child = snapshot.get_new_instance(*new_child_ref).unwrap(); if let Some(old_child) = old_child_map.remove(new_child.name.as_str()) { @@ -225,6 +246,12 @@ pub fn syncback_dir_no_meta<'sync>( mod test { use super::*; + use std::path::PathBuf; + + use crate::{ + snapshot::{InstanceMetadata, InstanceSnapshot}, + Project, RojoTree, SyncbackData, SyncbackSnapshot, + }; use memofs::{InMemoryFs, VfsSnapshot}; #[test] @@ -261,4 +288,237 @@ mod test { insta::assert_yaml_snapshot!(instance_snapshot); } + + fn make_project() -> Project { + serde_json::from_str(r#"{"tree": {"$className": "DataModel"}}"#).unwrap() + } + + fn make_vfs() -> Vfs { + let mut imfs = InMemoryFs::new(); + imfs.load_snapshot("/root", VfsSnapshot::empty_dir()).unwrap(); + Vfs::new(imfs) + } + + /// Two children whose Roblox names are identical when lowercased ("Alpha" + /// and "alpha") but live at different filesystem paths because of the + /// `name` property ("Beta/" and "Alpha/" respectively). The dedup check + /// must use the actual filesystem paths, not the raw Roblox names, to + /// avoid a false-positive duplicate error. + #[test] + fn syncback_no_false_duplicate_with_name_prop() { + use rbx_dom_weak::{InstanceBuilder, WeakDom}; + + // Old child A: Roblox name "Alpha", on disk at "/root/Beta" + // (name property maps "Alpha" → "Beta" on the filesystem) + let old_child_a = InstanceSnapshot::new() + .name("Alpha") + .class_name("Folder") + .metadata( + InstanceMetadata::new() + .instigating_source(PathBuf::from("/root/Beta")) + .relevant_paths(vec![PathBuf::from("/root/Beta")]), + ); + // Old child B: Roblox name "alpha", on disk at "/root/Alpha" + let old_child_b = InstanceSnapshot::new() + .name("alpha") + .class_name("Folder") + .metadata( + InstanceMetadata::new() + .instigating_source(PathBuf::from("/root/Alpha")) + .relevant_paths(vec![PathBuf::from("/root/Alpha")]), + ); + let old_parent = InstanceSnapshot::new() + .name("Parent") + .class_name("Folder") + .children(vec![old_child_a, old_child_b]) + .metadata( + InstanceMetadata::new() + .instigating_source(PathBuf::from("/root")) + .relevant_paths(vec![PathBuf::from("/root")]), + ); + let old_tree = RojoTree::new(old_parent); + + // New state: same two children in Roblox. + let mut new_tree = WeakDom::new(InstanceBuilder::new("ROOT")); + let new_parent = new_tree.insert( + new_tree.root_ref(), + InstanceBuilder::new("Folder").with_name("Parent"), + ); + new_tree.insert(new_parent, InstanceBuilder::new("Folder").with_name("Alpha")); + new_tree.insert(new_parent, InstanceBuilder::new("Folder").with_name("alpha")); + + let vfs = make_vfs(); + let project = make_project(); + let data = SyncbackData::for_test(&vfs, &old_tree, &new_tree, &project); + let snapshot = SyncbackSnapshot { + data, + old: Some(old_tree.get_root_id()), + new: new_parent, + path: PathBuf::from("/root"), + middleware: None, + }; + + let result = syncback_dir_no_meta(&snapshot); + assert!( + result.is_ok(), + "should not error when two children have the same lowercased Roblox \ + name but map to distinct filesystem paths: {result:?}", + ); + } + + /// Two completely new children with the same non-init name would produce + /// the same filesystem entry and must be detected as a duplicate. + #[test] + fn syncback_detects_sibling_duplicate_names() { + use rbx_dom_weak::{InstanceBuilder, WeakDom}; + + let old_parent = InstanceSnapshot::new() + .name("Parent") + .class_name("Folder") + .metadata( + InstanceMetadata::new() + .instigating_source(PathBuf::from("/root")) + .relevant_paths(vec![PathBuf::from("/root")]), + ); + let old_tree = RojoTree::new(old_parent); + + let mut new_tree = WeakDom::new(InstanceBuilder::new("ROOT")); + let new_parent = new_tree.insert( + new_tree.root_ref(), + InstanceBuilder::new("Folder").with_name("Parent"), + ); + // "Foo" is not a reserved name but two siblings named "Foo" still + // collide on disk. + new_tree.insert(new_parent, InstanceBuilder::new("Folder").with_name("Foo")); + new_tree.insert(new_parent, InstanceBuilder::new("Folder").with_name("Foo")); + + let vfs = make_vfs(); + let project = make_project(); + let data = SyncbackData::for_test(&vfs, &old_tree, &new_tree, &project); + let snapshot = SyncbackSnapshot { + data, + old: Some(old_tree.get_root_id()), + new: new_parent, + path: PathBuf::from("/root"), + middleware: None, + }; + + let result = syncback_dir_no_meta(&snapshot); + assert!( + result.is_err(), + "should error when two new children would produce the same filesystem name", + ); + } + + /// A new child named "Init" (as a ModuleScript) would naively become + /// "Init.luau", which case-insensitively matches the parent's reserved + /// "init.luau". Syncback must resolve this automatically by prefixing the + /// filesystem name with '_' (→ "_Init.luau") rather than erroring. + #[test] + fn syncback_resolves_init_name_conflict() { + use rbx_dom_weak::{InstanceBuilder, WeakDom}; + + let old_parent = InstanceSnapshot::new() + .name("Parent") + .class_name("Folder") + .metadata( + InstanceMetadata::new() + .instigating_source(PathBuf::from("/root")) + .relevant_paths(vec![PathBuf::from("/root")]), + ); + let old_tree = RojoTree::new(old_parent); + + let mut new_tree = WeakDom::new(InstanceBuilder::new("ROOT")); + let new_parent = new_tree.insert( + new_tree.root_ref(), + InstanceBuilder::new("Folder").with_name("Parent"), + ); + new_tree.insert( + new_parent, + InstanceBuilder::new("ModuleScript").with_name("Init"), + ); + + let vfs = make_vfs(); + let project = make_project(); + let data = SyncbackData::for_test(&vfs, &old_tree, &new_tree, &project); + let snapshot = SyncbackSnapshot { + data, + old: Some(old_tree.get_root_id()), + new: new_parent, + path: PathBuf::from("/root"), + middleware: None, + }; + + let result = syncback_dir_no_meta(&snapshot); + assert!( + result.is_ok(), + "should resolve init-name conflict by prefixing '_', not error: {result:?}", + ); + // The child should have been placed at "_Init.luau", not "Init.luau". + let child_file_name = result + .unwrap() + .children + .into_iter() + .next() + .and_then(|c| c.path.file_name().map(|n| n.to_string_lossy().into_owned())) + .unwrap_or_default(); + assert!( + child_file_name.starts_with('_'), + "child filesystem name should start with '_' to avoid init collision, \ + got: {child_file_name}", + ); + } + + /// A child whose filesystem name is stored with a slugified prefix (e.g. + /// "_Init") must NOT be blocked — only the bare "init" stem is reserved. + #[test] + fn syncback_allows_slugified_init_name() { + use rbx_dom_weak::{InstanceBuilder, WeakDom}; + + // Existing child: on disk as "_Init" (slugified from a name with an + // illegal character), its stem is "_init" which is not reserved. + let old_child = InstanceSnapshot::new() + .name("Init") + .class_name("Folder") + .metadata( + InstanceMetadata::new() + .instigating_source(PathBuf::from("/root/_Init")) + .relevant_paths(vec![PathBuf::from("/root/_Init")]), + ); + let old_parent = InstanceSnapshot::new() + .name("Parent") + .class_name("Folder") + .children(vec![old_child]) + .metadata( + InstanceMetadata::new() + .instigating_source(PathBuf::from("/root")) + .relevant_paths(vec![PathBuf::from("/root")]), + ); + let old_tree = RojoTree::new(old_parent); + + let mut new_tree = WeakDom::new(InstanceBuilder::new("ROOT")); + let new_parent = new_tree.insert( + new_tree.root_ref(), + InstanceBuilder::new("Folder").with_name("Parent"), + ); + new_tree.insert(new_parent, InstanceBuilder::new("Folder").with_name("Init")); + + let vfs = make_vfs(); + let project = make_project(); + let data = SyncbackData::for_test(&vfs, &old_tree, &new_tree, &project); + let snapshot = SyncbackSnapshot { + data, + old: Some(old_tree.get_root_id()), + new: new_parent, + path: PathBuf::from("/root"), + middleware: None, + }; + + let result = syncback_dir_no_meta(&snapshot); + assert!( + result.is_ok(), + "should allow a child whose filesystem name is slugified away from \ + the reserved 'init' stem: {result:?}", + ); + } } diff --git a/src/snapshot_middleware/meta_file.rs b/src/snapshot_middleware/meta_file.rs index 3a6a6fda..81cbe0cb 100644 --- a/src/snapshot_middleware/meta_file.rs +++ b/src/snapshot_middleware/meta_file.rs @@ -154,11 +154,18 @@ impl AdjacentMetadata { .old_inst() .and_then(|inst| inst.metadata().specified_name.clone()) .or_else(|| { - // If this is a new instance and its name is invalid for the filesystem, - // we need to specify the name in meta.json so it can be preserved + // Write name when the filesystem path doesn't match the + // instance name (invalid chars or init-prefix). if snapshot.old_inst().is_none() { let instance_name = &snapshot.new_inst().name; - if validate_file_name(instance_name).is_err() { + let fs_stem = path + .file_name() + .and_then(|n| n.to_str()) + .map(|s| s.split('.').next().unwrap_or(s)) + .unwrap_or(""); + if validate_file_name(instance_name).is_err() + || fs_stem != instance_name.as_str() + { Some(instance_name.clone()) } else { None @@ -421,11 +428,17 @@ impl DirectoryMetadata { .old_inst() .and_then(|inst| inst.metadata().specified_name.clone()) .or_else(|| { - // If this is a new instance and its name is invalid for the filesystem, - // we need to specify the name in meta.json so it can be preserved + // Write name when the directory name doesn't match the + // instance name (invalid chars or init-prefix). if snapshot.old_inst().is_none() { let instance_name = &snapshot.new_inst().name; - if validate_file_name(instance_name).is_err() { + let fs_name = path + .file_name() + .and_then(|n| n.to_str()) + .unwrap_or(""); + if validate_file_name(instance_name).is_err() + || fs_name != instance_name.as_str() + { Some(instance_name.clone()) } else { None diff --git a/src/syncback/file_names.rs b/src/syncback/file_names.rs index ff867a83..c1d0d459 100644 --- a/src/syncback/file_names.rs +++ b/src/syncback/file_names.rs @@ -36,23 +36,33 @@ pub fn name_for_inst<'a>( | Middleware::ServerScriptDir | Middleware::ClientScriptDir | Middleware::ModuleScriptDir => { - if validate_file_name(&new_inst.name).is_err() { + let name = if validate_file_name(&new_inst.name).is_err() { Cow::Owned(slugify_name(&new_inst.name)) } else { - Cow::Borrowed(&new_inst.name) + Cow::Borrowed(new_inst.name.as_str()) + }; + // Prefix "init" to avoid colliding with reserved init files. + if name.to_lowercase() == "init" { + Cow::Owned(format!("_{name}")) + } else { + name } } _ => { let extension = extension_for_middleware(middleware); let slugified; - let final_name = if validate_file_name(&new_inst.name).is_err() { + let stem: &str = if validate_file_name(&new_inst.name).is_err() { slugified = slugify_name(&new_inst.name); &slugified } else { &new_inst.name }; - - Cow::Owned(format!("{final_name}.{extension}")) + // Prefix "init" stems to avoid colliding with reserved init files. + if stem.to_lowercase() == "init" { + Cow::Owned(format!("_{stem}.{extension}")) + } else { + Cow::Owned(format!("{stem}.{extension}")) + } } }) } diff --git a/src/syncback/snapshot.rs b/src/syncback/snapshot.rs index 384033d6..dfdf1a62 100644 --- a/src/syncback/snapshot.rs +++ b/src/syncback/snapshot.rs @@ -237,6 +237,24 @@ pub fn inst_path(dom: &WeakDom, referent: Ref) -> String { path.join("/") } +impl<'sync> SyncbackData<'sync> { + /// Constructs a `SyncbackData` for use in unit tests. + #[cfg(test)] + pub fn for_test( + vfs: &'sync Vfs, + old_tree: &'sync RojoTree, + new_tree: &'sync WeakDom, + project: &'sync Project, + ) -> Self { + Self { + vfs, + old_tree, + new_tree, + project, + } + } +} + #[cfg(test)] mod test { use rbx_dom_weak::{InstanceBuilder, WeakDom};