Repository navigation
Conversation
CryZe
left a comment
There was a problem hiding this comment.
The public-profile API and the attachment tests look good. Before merging, please replace the two new module-wide dead-code suppressions with conditional compilation of the code that is only needed for automatic detection or tests. The public profile constants and explicit attachment must remain available without alloc. I'll leave this change to you rather than push an overlapping fix.
| #[cfg_attr(not(feature = "alloc"), allow(dead_code))] | ||
| mod builds; | ||
| #[cfg(any(feature = "alloc", test))] | ||
| #[cfg_attr(not(feature = "alloc"), allow(dead_code))] |
There was a problem hiding this comment.
Please remove both #[cfg_attr(not(feature = "alloc"), allow(dead_code))] annotations. We want unused-code warnings to remain useful, and code that is deliberately unavailable in a feature configuration should not be compiled in that configuration.
The public constants do need these modules' profile tables without alloc, so gating the entire modules is not appropriate anymore. Instead, gate the lookup-only pieces with #[cfg(any(feature = "alloc", test))]: builds::nearest, builds::nearest_in, linux_builds::nearest, their conditional imports, and the Build::unity metadata/initializers if they are only used by lookup and tests. Keep the profile data and constants unconditional. Including test is important because the allocation-free unit tests inspect those tables.
I checked with RUSTFLAGS="--force-warn=dead_code" cargo check --no-default-features --features unity --lib: the scene-manager warnings are precisely the unity field and those three lookup helpers. Please verify the normal no-alloc library check is warning-free without these suppressions, and retain the tests both with and without alloc.
There was a problem hiding this comment.
I replaced them with #[cfg(any(feature = "alloc", test))] on the lookup functions and Build::unity in 7670200. The library check without alloc is warning-free now and the tests pass with and without it.
I copied those two annotations from Mono though. You added the same ones to mono/mod.rs in 87a26aa and they're still on master:
asr/src/game_engine/unity/mono/mod.rs
Lines 29 to 35 in 06f244e
Should I open a PR that gates Mono's lookup code the same way?
367a432 to
7670200
Compare
Part of #174.
The scene manager now has the same attach functions as Mono and IL2CPP.
SceneManager::attach(process, profile)andwait_attach(process, profile)take aProfile, and the automatic ones are nowattach_auto_detectandwait_attach_auto_detect. This renames the oldattach(process)andwait_attach(process).Profile,Anchor, the offset structs,PathShapeandReferenceShapeare public with public fields, so an auto splitter can write its own profile, signature included.profileshas a constant for each of the 29 Windows and 17 Linux builds.UnityPlayer.dllorUnityPlayer.soand doesn't needalloc. Withallocit also finds a game that has the engine in its executable.matchonPathShapeandReferenceShapeis back, since the public enums don't warn withoutallocanymore. A new variant is a compile error again.#133 had
attach_with_offsetsfor this. The module name stays in the crate for now. That's the next part of #174, for all three modules at once.cargo test --all-featuresandcargo test --no-default-features --features unity. The new tests pin every constant to its build and attach throughUnityPlayer.dll,UnityPlayer.soand an executable. I also ran all 93 Linux and Windows fixture players on it.