diff --git a/corlib/MenSharpBehaviour.Unity.cs b/corlib/MenSharpBehaviour.Unity.cs index 5d65a0b..6502a4d 100644 --- a/corlib/MenSharpBehaviour.Unity.cs +++ b/corlib/MenSharpBehaviour.Unity.cs @@ -25,6 +25,30 @@ public class MenSharpBehaviour public UnityEngine.Transform transform { get; } + private System.Threading.CancellationTokenSource destroyTokenSource; + + public System.Threading.CancellationToken destroyCancellationToken + { + get + { + // Allocate the source lazily, as Unity does. + if (destroyTokenSource == null) + { + destroyTokenSource = new System.Threading.CancellationTokenSource(); + } + return destroyTokenSource.Token; + } + } + + // Called before the user's OnDestroy by generated code. + internal void __CancelDestroyToken() + { + if (destroyTokenSource != null) + { + destroyTokenSource.Cancel(); + } + } + // The program itself. Declared as UdonBehaviour rather than as the // interface its methods live on, because a `this` heap reference may // only be a GameObject, a Transform or an UdonBehaviour — Udon refuses diff --git a/src/men-sharp-codegen/src/generator.rs b/src/men-sharp-codegen/src/generator.rs index 8ccbc26..8e4ff44 100644 --- a/src/men-sharp-codegen/src/generator.rs +++ b/src/men-sharp-codegen/src/generator.rs @@ -350,6 +350,8 @@ struct FieldCallback { struct EventEntry { name: String, key: FunctionKey, + /// Pre-hooks, in execution order, before the event body. + before: Vec, arguments: Vec, /// `OnOwnershipRequest`: Udon reads the result back from `__returnValue`. returns_value: bool, @@ -982,6 +984,7 @@ impl<'a, 'ast> Generator<'a, 'ast> { entries.push(EventEntry { name, key, + before: Vec::new(), arguments, // the one event whose *result* Udon reads back, from // `__returnValue` (UdonSharp does the same copy) @@ -1004,6 +1007,7 @@ impl<'a, 'ast> Generator<'a, 'ast> { } } } + self.add_destroy_token_handler(&mut entries, &mut claimed); // `[FieldChangeCallback]` fields each get an `_onVarChange_…` entry; // collected before the queue drains so their setters get compiled let callbacks = self.collect_field_callbacks(); @@ -1057,6 +1061,7 @@ impl<'a, 'ast> Generator<'a, 'ast> { entries.push(EventEntry { name: delegates::REMOTE_INVOKE_EVENT.to_string(), key, + before: Vec::new(), arguments: Vec::new(), returns_value: false, result_slot: None, @@ -1085,6 +1090,22 @@ impl<'a, 'ast> Generator<'a, 'ast> { let key = &entry.key; self.begin_entry_stub(name, init_label, init_return, initialized); + for (index, before) in entry.before.iter().enumerate() { + let function = &self.functions[before]; + let (return_slot, label) = (function.return_slot, function.label); + // Resume at the next hook, or the event body, after this call. + let done = self + .program + .add_label(format!("event_{name}__before_{index}_done")); + let return_to_stub = + self.code_address_constant(format!("__ret_before_{name}_{index}"), Some(done)); + self.copy(return_to_stub, return_slot); + self.program.code.push(Op::Jump(Target::Label(label))); + self.program.code.push(Op::Label(done)); + // Do not continue into the event after an uncaught hook error. + self.emit_unhandled_check(name); + } + // the event's arguments: the runtime wrote them into the named // slots before raising the event; hand them to the function let function = &self.functions[key]; @@ -2795,6 +2816,7 @@ impl<'a, 'ast> Generator<'a, 'ast> { entries.push(EventEntry { name, key, + before: Vec::new(), arguments, returns_value: false, result_slot: None, @@ -2804,6 +2826,63 @@ impl<'a, 'ast> Generator<'a, 'ast> { added } + /// Adds MenSharpBehaviour's cancellation hook to Udon's `OnDestroy` + /// event, without taking the event away from a user-defined handler. + fn add_destroy_token_handler( + &mut self, + entries: &mut Vec, + claimed: &mut HashSet, + ) { + let Some(marker) = self.marker else { + return; + }; + // The marker owns the hook; the source subclass owns `OnDestroy`. + let Some(method) = self + .declarations + .table + .symbol(marker) + .members_named("__CancelDestroyToken") + .iter() + .copied() + .find(|&member| self.declarations.table.symbol(member).kind == SymbolKind::Method) + else { + return; + }; + let key = FunctionKey { + symbol: method, + role: Role::Method, + bindings: Vec::new(), + }; + self.add_before_handler(entries, claimed, udon_event_name("OnDestroy"), key); + } + + /// Adds a pre-hook, or creates a synthetic entry when no body exists. + fn add_before_handler( + &mut self, + entries: &mut Vec, + claimed: &mut HashSet, + name: String, + key: FunctionKey, + ) { + // Compile the hook even when it is the synthetic event body. + self.ensure_function(&key); + if let Some(entry) = entries.iter_mut().find(|entry| entry.name == name) { + // Keep registration order so hooks run deterministically. + entry.before.push(key); + return; + } + // With no user body, the first hook is the entry's main function. + claimed.insert(name.clone()); + entries.push(EventEntry { + name, + key, + before: Vec::new(), + arguments: Vec::new(), + returns_value: false, + result_slot: None, + }); + } + fn find_symbol(&self, path: &[&str]) -> Option { let mut current = self.declarations.table.root(); for segment in path { @@ -3978,15 +4057,7 @@ impl<'a, 'ast> Generator<'a, 'ast> { if self.is_bodiless(symbol) { return false; } - let entry = self.declarations.table.symbol(symbol); - entry.declarations.iter().all(|site| { - matches!(&site.syntax, SyntaxRef::Property(property) - if matches!(&property.body, FunctionBody::Accessors(accessors) - if accessors.accessors.iter().all(|accessor| matches!( - accessor.body, - FunctionBody::None { .. } - )))) - }) + self.declarations.table.symbol(symbol).is_auto_property() } fn collect_statics(&mut self, class: SymbolId, export: bool) { @@ -4048,6 +4119,7 @@ impl<'a, 'ast> Generator<'a, 'ast> { entries.push(EventEntry { name: layout.event, key, + before: Vec::new(), arguments, returns_value: false, result_slot, @@ -4418,8 +4490,8 @@ impl<'a, 'ast> Generator<'a, 'ast> { member } - /// The heap slot for a member declared directly on `MenSharpBehaviour` - /// (`gameObject`, `transform`), or `None` for anything else. + /// The heap slot for an auto-property declared directly on + /// `MenSharpBehaviour`, or `None` for anything else. /// /// Udon has no `this` and no extern that returns a program's own object, /// so these are not calls: each gets a private slot whose initial value is @@ -4433,6 +4505,9 @@ impl<'a, 'ast> Generator<'a, 'ast> { if symbol.parent != Some(marker) { return None; } + if !self.is_auto_property(member) { + return None; + } if let Some(&slot) = self.statics.get(&member) { return Some(slot); } diff --git a/src/men-sharp-codegen/src/generator/exceptions.rs b/src/men-sharp-codegen/src/generator/exceptions.rs index f814ad6..afab99e 100644 --- a/src/men-sharp-codegen/src/generator/exceptions.rs +++ b/src/men-sharp-codegen/src/generator/exceptions.rs @@ -412,7 +412,9 @@ impl<'a, 'ast> Generator<'a, 'ast> { self.ensure_function(&key); let function = &self.functions[&key]; let (label, return_slot) = (function.label, function.return_slot); - let halt = self.code_address_constant(format!("__halt_after_{name}"), None); + let halt = + self.code_address_constant(format!("__halt_after_{name}_{}", self.temp_counter), None); + self.temp_counter += 1; self.copy(halt, return_slot); self.program.code.push(Op::Jump(Target::Label(label))); self.program.code.push(Op::Label(ok)); diff --git a/src/men-sharp-compiler/tests/codegen.rs b/src/men-sharp-compiler/tests/codegen.rs index aaf94d6..ae3f4d6 100644 --- a/src/men-sharp-compiler/tests/codegen.rs +++ b/src/men-sharp-compiler/tests/codegen.rs @@ -10822,6 +10822,126 @@ fn cancellation_stops_a_waiting_method_at_its_await() { ); } +#[test] +fn destroy_cancellation_token_is_lazy_and_cancelled_before_the_user_handler() { + let mut sources = vec![SourceCode::new( + "base.cs", + r#" + namespace MenSharp + { + public class MenSharpBehaviour + { + private System.Threading.CancellationTokenSource destroyTokenSource; + public bool destroyTokenSourceCreated; + public System.Threading.CancellationToken destroyCancellationToken + { + get + { + if (destroyTokenSource == null) + { + destroyTokenSource = new System.Threading.CancellationTokenSource(); + destroyTokenSourceCreated = true; + } + return destroyTokenSource.Token; + } + } + internal void __CancelDestroyToken() + { + if (destroyTokenSource != null) + destroyTokenSource.Cancel(); + } + } + } + "#, + )]; + sources.push(SourceCode::new( + "test.cs", + r#" + using MenSharp; + public class Probe : MenSharpBehaviour + { + public bool cancelled; + public bool observedBeforeDestroy; + public bool cached; + public void Start() + { + observedBeforeDestroy = destroyTokenSourceCreated; + cached = destroyCancellationToken.CanBeCanceled; + } + public void OnDestroy() { cancelled = destroyCancellationToken.IsCancellationRequested; } + } + "#, + )); + sources.extend( + Compiler::corlib_sources() + .into_iter() + .filter(|source| source.name.as_ref() != "corlib/MenSharpBehaviour.cs"), + ); + let Some(program) = compile_behaviour(sources, "Probe") else { + eprintln!("skipped: no .NET runtime"); + return; + }; + assert!( + program.output.errors.is_empty(), + "{:#?}", + program.output.errors + ); + assert!( + !program + .output + .program + .data + .iter() + .any(|symbol| symbol.name == "__this_destroyCancellationToken") + ); + // Unity's symbol table rejects duplicate heap names. + let mut data_names = std::collections::HashSet::new(); + assert!( + program + .output + .program + .data + .iter() + .all(|symbol| data_names.insert(&symbol.name)), + "generated data symbols must have unique names" + ); + + let assembled = program.output.program.assemble().unwrap(); + // An uncached token is created by OnDestroy itself, after the pre-hook. + let mut uncached = Emulator::new(&program.output.program, &assembled); + uncached.run(&assembled, "_onDestroy").unwrap(); + assert!(matches!( + uncached.value_of("cancelled"), + Some(Value::Boolean(false)) + )); + assert!(matches!( + uncached.value_of("destroyTokenSourceCreated"), + Some(Value::Boolean(true)) + )); + + // A cached token is canceled by the pre-hook before OnDestroy runs. + let mut emulator = Emulator::new(&program.output.program, &assembled); + // Start is a built-in Udon entry point; arbitrary public methods are not. + emulator.run(&assembled, "_start").unwrap(); + assert!(matches!( + emulator.value_of("observedBeforeDestroy"), + Some(Value::Boolean(false)) + )); + assert!(matches!( + emulator.value_of("cached"), + Some(Value::Boolean(true)) + )); + emulator.run(&assembled, "_onDestroy").unwrap(); + assert!(matches!( + emulator.value_of("cancelled"), + Some(Value::Boolean(true)) + )); + assert!(matches!( + emulator.value_of("destroyTokenSourceCreated"), + Some(Value::Boolean(true)) + )); +} + #[test] fn iterator_misuse_is_rejected_by_the_checker() { let Some(errors) = body_errors( diff --git a/src/men-sharp-semantics/src/symbol.rs b/src/men-sharp-semantics/src/symbol.rs index e0a54c6..4d592ed 100644 --- a/src/men-sharp-semantics/src/symbol.rs +++ b/src/men-sharp-semantics/src/symbol.rs @@ -262,6 +262,26 @@ impl<'ast> Symbol<'ast> { self.member_map.get(name).map(Vec::as_slice).unwrap_or(&[]) } + /// Whether this source symbol declares an auto-property whose accessors + /// have no bodies, such as `{ get; }` or `{ get; set; }`. + pub fn is_auto_property(&self) -> bool { + self.kind == SymbolKind::Property + && self.declarations.iter().all(|declaration| { + matches!( + &declaration.syntax, + SyntaxRef::Property(property) + if matches!( + &property.body, + FunctionBody::Accessors(accessors) + if accessors.accessors.iter().all(|accessor| matches!( + accessor.body, + FunctionBody::None { .. } + )) + ) + ) + }) + } + pub fn member_names(&self) -> impl Iterator { self.member_map .iter() diff --git a/tests/unity-project/Assets/MenSharp/MenSharpRuntimeSmoke.cs b/tests/unity-project/Assets/MenSharp/MenSharpRuntimeSmoke.cs index aa0dd83..72bd54d 100644 --- a/tests/unity-project/Assets/MenSharp/MenSharpRuntimeSmoke.cs +++ b/tests/unity-project/Assets/MenSharp/MenSharpRuntimeSmoke.cs @@ -15,6 +15,24 @@ public class MenSharpRuntimeSmoke : MenSharpBehaviour public int syncResult; public string syncText; + // OnDestroy cancels a token that was requested before destruction. + public bool cacheDestroyToken; + public bool destroyTokenCached; + public bool destroyTokenCanceled; + + public void Start() + { + if (cacheDestroyToken) + { + destroyTokenCached = destroyCancellationToken.CanBeCanceled; + } + } + + public void OnDestroy() + { + destroyTokenCanceled = destroyCancellationToken.IsCancellationRequested; + } + public bool asyncDone; public int asyncResult; diff --git a/tests/unity-project/Assets/Tests/Editor/MenSharpCancellationTokenTests.cs b/tests/unity-project/Assets/Tests/Editor/MenSharpCancellationTokenTests.cs new file mode 100644 index 0000000..dcd2105 --- /dev/null +++ b/tests/unity-project/Assets/Tests/Editor/MenSharpCancellationTokenTests.cs @@ -0,0 +1,95 @@ +#if UNITY_EDITOR +using System.Collections; +using System.Collections.Generic; +using NUnit.Framework; +using UnityEditor; +using UnityEditor.SceneManagement; +using UnityEngine; +using UnityEngine.TestTools; +using VRC.Udon; + +public class MenSharpCancellationTokenTests +{ + private const string GeneratedScene = "Assets/Tests/Generated/MenSharpCancellationToken.unity"; + + [UnityTest] + public IEnumerator DestroyCancellationTokenIsLazyAndCanceledBeforeOnDestroy() + { + AssetDatabase.Refresh(); + if (MenSharpTestScene.ScriptReloadPending()) + { + yield return new WaitForDomainReload(); + } + + MenSharpCompiler.RebuildAll(); + AssetDatabase.Refresh(ImportAssetOptions.ForceSynchronousImport); + BuildScene(); + + yield return new EnterPlayMode(); + yield return null; + yield return null; + Assert.IsTrue(Application.isPlaying, "the editor did not enter play mode"); + + UdonBehaviour cached = MenSharpTestScene.FindUdon("MenSharpCancellationCached"); + UdonBehaviour uncached = MenSharpTestScene.FindUdon("MenSharpCancellationUncached"); + Assert.IsTrue(cached.IsInitialized, "the cached behaviour was not initialised"); + Assert.IsTrue(uncached.IsInitialized, "the uncached behaviour was not initialised"); + + // Start creates the source only for the behaviour that caches its token. + cached.RunProgram("Start"); + uncached.RunProgram("Start"); + Assert.AreEqual(true, cached.GetProgramVariable("destroyTokenCached")); + Assert.AreEqual(false, uncached.GetProgramVariable("destroyTokenCached")); + + // The generated pre-hook cancels before the user OnDestroy handler. + cached.RunProgram("_onDestroy"); + Assert.AreEqual(true, cached.GetProgramVariable("destroyTokenCanceled"), + "the cached token was not canceled before OnDestroy"); + uncached.RunProgram("_onDestroy"); + Assert.AreEqual(false, uncached.GetProgramVariable("destroyTokenCanceled")); + + yield return new ExitPlayMode(); + AssetDatabase.DeleteAsset(GeneratedScene); + } + + [UnityTearDown] + public IEnumerator LeavePlayModeAfterAFailure() + { + if (Application.isPlaying) + { + yield return new ExitPlayMode(); + } + AssetDatabase.DeleteAsset(GeneratedScene); + } + + private static void BuildScene() + { + MenSharpTestScene.EnsureFolder("Assets/Tests/Generated"); + var scene = EditorSceneManager.NewScene(NewSceneSetup.EmptyScene, NewSceneMode.Single); + + GameObject cached = MenSharpTestScene.AddProxy( + "MenSharpCancellationCached", "MenSharpRuntimeSmoke", Vector3.zero); + MenSharpTestScene.Assign( + MenSharpTestScene.Proxy(cached, "MenSharpRuntimeSmoke"), + "cacheDestroyToken", + true); + + GameObject uncached = MenSharpTestScene.AddProxy( + "MenSharpCancellationUncached", "MenSharpRuntimeSmoke", Vector3.up * 4); + MenSharpTestScene.Assign( + MenSharpTestScene.Proxy(uncached, "MenSharpRuntimeSmoke"), + "cacheDestroyToken", + false); + + var targets = new List { cached, uncached }; + MenSharpProxy.SyncThenTransfer(targets, false); + foreach (GameObject target in targets) + { + Assert.IsNotNull(target.GetComponent(), target.name); + } + + AssetDatabase.DeleteAsset(GeneratedScene); + Assert.IsTrue(EditorSceneManager.SaveScene(scene, GeneratedScene)); + } +} +#endif diff --git a/tests/unity-project/Assets/Tests/Editor/MenSharpCancellationTokenTests.cs.meta b/tests/unity-project/Assets/Tests/Editor/MenSharpCancellationTokenTests.cs.meta new file mode 100644 index 0000000..dbc6a0d --- /dev/null +++ b/tests/unity-project/Assets/Tests/Editor/MenSharpCancellationTokenTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 9b5c3dd1a7754d6b8e3f4a1c2d607e91 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/tools/test-unity.bat b/tools/test-unity.bat new file mode 100644 index 0000000..866c39a --- /dev/null +++ b/tools/test-unity.bat @@ -0,0 +1,121 @@ +@ECHO OFF +SETLOCAL ENABLEEXTENSIONS + +:: Runs the SDK-backed integration suite without opening the Unity UI - the +:: same thing CI does, against the local Unity install. +:: +:: tools\test-unity.bat [nunit test filter] +:: +:: Environment: +:: UNITY_EDITOR the Unity 2022.3.22f1 binary (default: Unity Hub's) +:: MENSHARP_VRC_PROJECT the Worlds project to borrow the SDK from (default: +:: %USERPROFILE%\ALCOM\Projects\MenSharpTest when +:: vrc-get is absent) +:: +:: Results land in artifacts\unity-tests: results.xml (NUnit) and Editor.log. + +FOR %%I IN ("%~dp0..") DO SET "repo=%%~fI" +SET "project=%repo%\tests\unity-project" +SET "artifacts=%repo%\artifacts\unity-tests" +IF DEFINED UNITY_EDITOR ( + SET "unity=%UNITY_EDITOR%" +) ELSE ( + SET "unity=%ProgramFiles%\Unity\Hub\Editor\2022.3.22f1\Editor\Unity.exe" +) + +SET "filter=%~1" +SET "packages=%project%\Packages" +SET "mensharp=%packages%\io.tesca.mensharp" +IF NOT EXIST "%packages%\" ( + ECHO error: %project% does not look like a Unity project (no Packages/^) 1>&2 + EXIT /B 1 +) + +IF NOT EXIST "%unity%" ( + ECHO Unity editor not found at %unity%; set UNITY_EDITOR 1>&2 + EXIT /B 1 +) + +IF NOT "%MENSHARP_VRC_PROJECT%"=="" GOTO STAGE_PACKAGE +WHERE vrc-get >NUL 2>&1 +IF NOT ERRORLEVEL 1 GOTO STAGE_PACKAGE +SET "default_donor=%USERPROFILE%\ALCOM\Projects\MenSharpTest" +IF EXIST "%default_donor%\Packages\com.vrchat.worlds\" SET "MENSHARP_VRC_PROJECT=%default_donor%" + +:STAGE_PACKAGE +ECHO building the compiler (release)... +cargo build --release -p men-sharp --manifest-path "%repo%\Cargo.toml" +IF ERRORLEVEL 1 EXIT /B 1 + +IF EXIST "%mensharp%\" RD /S /Q "%mensharp%" +IF ERRORLEVEL 1 EXIT /B 1 +MKDIR "%mensharp%" +IF ERRORLEVEL 1 EXIT /B 1 +XCOPY "%repo%\unity\io.tesca.mensharp\*" "%mensharp%\" /E /I /Y /H /R /K >NUL +IF ERRORLEVEL 2 EXIT /B 1 +IF NOT EXIST "%mensharp%\Compiler~\" MKDIR "%mensharp%\Compiler~" +IF ERRORLEVEL 1 EXIT /B 1 +COPY /Y "%repo%\target\release\men-sharp.exe" "%mensharp%\Compiler~\men-sharp-windows-x64.exe" >NUL +IF ERRORLEVEL 1 EXIT /B 1 + +IF "%MENSHARP_SKIP_VPM_RESOLVE%"=="1" GOTO SDK_READY +WHERE vrc-get >NUL 2>&1 +IF NOT ERRORLEVEL 1 GOTO RESOLVE_SDK +IF "%MENSHARP_VRC_PROJECT%"=="" ( + ECHO vrc-get is not installed and MENSHARP_VRC_PROJECT is unset 1>&2 + ECHO set MENSHARP_VRC_PROJECT to an ALCOM/VCC Worlds project 1>&2 + EXIT /B 1 +) +SET "donor=%MENSHARP_VRC_PROJECT%" +FOR %%N IN (com.vrchat.base com.vrchat.worlds) DO ( + IF NOT EXIST "%donor%\Packages\%%N\" ( + ECHO missing %donor%\Packages\%%N 1>&2 + EXIT /B 1 + ) + IF EXIST "%packages%\%%N\" RD /S /Q "%packages%\%%N" + IF ERRORLEVEL 1 EXIT /B 1 + MKLINK /J "%packages%\%%N" "%donor%\Packages\%%N" >NUL + IF ERRORLEVEL 1 EXIT /B 1 +) +GOTO SDK_READY + +:RESOLVE_SDK +vrc-get resolve --project "%project%" +IF ERRORLEVEL 1 EXIT /B 1 + +:SDK_READY +IF EXIST "%artifacts%\" RD /S /Q "%artifacts%" +IF ERRORLEVEL 1 EXIT /B 1 +MKDIR "%artifacts%" +IF ERRORLEVEL 1 EXIT /B 1 + +IF NOT "%filter%"=="" GOTO RUN_FILTERED +"%unity%" -batchmode -nographics -projectPath "%project%" -runTests -testPlatform EditMode -assemblyNames ProjectTesca.MenSharp.IntegrationTests -testResults "%artifacts%\results.xml" -logFile "%artifacts%\Editor.log" +SET "status=%ERRORLEVEL%" +GOTO CHECK_RESULTS + +:RUN_FILTERED +"%unity%" -batchmode -nographics -projectPath "%project%" -runTests -testPlatform EditMode -assemblyNames ProjectTesca.MenSharp.IntegrationTests -testResults "%artifacts%\results.xml" -logFile "%artifacts%\Editor.log" -testFilter "%filter%" +SET "status=%ERRORLEVEL%" + +:CHECK_RESULTS +IF NOT EXIST "%artifacts%\results.xml" GOTO NO_RESULTS +FOR %%I IN ("%artifacts%\results.xml") DO IF %%~zI EQU 0 GOTO NO_RESULTS +FINDSTR /R /C:"NUL +IF ERRORLEVEL 1 GOTO TESTS_FAILED +ECHO Unity tests passed: %artifacts%\results.xml +EXIT /B 0 + +:NO_RESULTS +ECHO Unity produced no test results (exit %status%); see %artifacts%\Editor.log 1>&2 +IF EXIST "%artifacts%\Editor.log" FINDSTR /R /C:"error CS" /C:"Exception" /C:"Assertion" "%artifacts%\Editor.log" +EXIT /B 1 + +:TESTS_FAILED +ECHO Unity tests did not all pass (exit %status%); see %artifacts%\results.xml 1>&2 +FINDSTR /C:"&2 +EXIT /B 1