Repository navigation
Run Unity 2D projects as Codename One applications - #5976
Conversation
A Unity project's Assets and ProjectSettings in common/src/main/unity build unmodified: the .NET SDK compiles the C# scripts, maven/cil-translator turns the assembly into Java class files and compiles every scene, prefab, animator controller, clip, tile map and particle system to reflection-free Java, and maven/unity-compat is the runtime they run against (a .NET base library, the UnityEngine API over com.codename1.gaming, and our own TextMesh Pro and Cinemachine implementations). Maven (compile-unity, import-unity-project) and Gradle (compileUnity) share UnityProjectBuilder. Both modules sit behind the unity-compat profile and are published by the release. Float results have to match C# on every target, so native builds get -ffp-contract=off and the JavaScript target rounds float results with Math.fround. Bugs this uncovered and fixes outside the new modules, each with a test: - ParparVM JavaScript: wide stack shuffles, MULTIANEWARRAY dimension pops, a call result compared with FCMP/DCMP evaluated more than once, SWAP losing the lower value. - ParparVM native: arraycopy/clone of an array of primitive arrays took the primitive path. - GameView: a key repeat released a held key; a focused hosted child kept the multi-touch arrays from the view. - Box2D: chain against chain threw in World.popContact; the broad phase allocated a merge buffer on every step above 31 pairs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Developer Guide build artifacts are available for download from this workflow run:
Developer Guide quality checks: |
|
Compared 193 screenshots: 193 matched. Benchmark ResultsDetailed Performance Metrics
|
|
Compared 178 screenshots: 178 matched. Native Android coverage
✅ Native Android screenshot tests passed. Native Android coverage
Benchmark ResultsDetailed Performance Metrics
|
Cloudflare Preview
|
✅ Continuous Quality ReportTest & Coverage
Static Analysis
Generated automatically by the PR CI workflow. |
|
Compared 193 screenshots: 193 matched. Benchmark ResultsDetailed Performance Metrics
ParparVM vs HotSpot (JDK 25): windows-x64The performance gate could not run: the gate's own unit tests (test_perf_gate.py) failed. See |
|
Compared 193 screenshots: 193 matched. ParparVM vs HotSpot (JDK 25): linux-x64The performance gate could not run: the gate's own unit tests (test_perf_gate.py) failed. See |
|
Compared 193 screenshots: 193 matched. ParparVM vs HotSpot (JDK 25): linux-arm64The performance gate could not run: the gate's own unit tests (test_perf_gate.py) failed. See |
|
Compared 193 screenshots: 193 matched. Benchmark ResultsDetailed Performance Metrics
ParparVM vs HotSpot (JDK 25): windows-arm64The performance gate could not run: the gate's own unit tests (test_perf_gate.py) failed. See |
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
✅ ByteCodeTranslator Quality ReportTest & Coverage
Benchmark Results
Static Analysis
Generated automatically by the PR CI workflow. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5639acb341
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| UnityRuntime.begin(); | ||
|
|
||
| view = new UnityGameView(); |
There was a problem hiding this comment.
Calling UnityRuntime.begin() before the UnityGameView is created and initialized runs every script's Awake/OnEnable while PlayerPrefs still uses its transient in-memory fallback and AudioSource has no output. Consequently, saved preferences read in Awake appear missing, writes made there disappear once the store is installed, and every scene AudioSource with Play On Awake marks itself playing without ever sending audio to the device. Initialize these view-backed services before waking the first scene.
Useful? React with 👍 / 👎.
| if (negative) { | ||
| value = -value; |
There was a problem hiding this comment.
For the valid input "-2147483648", negating the scanned magnitude produces Long.MIN_VALUE, which is also used as the invalid-input sentinel. Parse therefore throws FormatException, and TryParse returns false, instead of producing Integer.MIN_VALUE. Use a distinct parse-status indicator so the minimum Int32 remains representable.
Useful? React with 👍 / 👎.
| c.resumeAt = 0f; | ||
| c.phase = 0; |
There was a problem hiding this comment.
When StartCoroutine is called from Start or Update, it immediately advances the iterator and adds it with phase == 0 and resumeAt == 0; the same frame's later runCoroutines(0) then advances it again. Thus a first yield return null resumes in the current frame rather than the next frame as promised, so initialization or frame-based loops can execute twice in one frame. Record the frame at which a null/default yield may next resume.
Useful? React with 👍 / 👎.
| public static float get_timeSinceLevelLoad() { | ||
| return time; |
There was a problem hiding this comment.
get_timeSinceLevelLoad() returns the application-wide Time.time, but changeScene() never resets that clock. After loading a second scene, scripts therefore see time since application startup rather than time since the current scene was loaded, which breaks scene-local timers and spawn schedules. Track a separate scene-load timestamp or reset a dedicated level clock on every load.
Useful? React with 👍 / 👎.
| public static boolean get_isMobilePlatform() { | ||
| return false; |
There was a problem hiding this comment.
Application.isMobilePlatform is hard-coded to false, including when the generated application runs on Android or iOS. Unity scripts commonly use this property to enable touch controls, mobile layouts, or device-specific behavior, so those paths remain disabled on the very device builds this compatibility layer targets. Have the host install or derive the actual platform value instead of returning a constant.
Useful? React with 👍 / 👎.
| xvfb-run -a mvn deploy -Psign-artifacts -Dgpg.passphrase=$MAVEN_GPG_PASSPHRASE \ | ||
| -Plocal-dev-javase -Darchetype.test.skip=true -DskipTests \ | ||
| -Plocal-dev-javase -Dunity-compat -Darchetype.test.skip=true -DskipTests \ |
There was a problem hiding this comment.
The release step first runs update-version.sh, whose mvn versions:set invocation does not activate unity-compat, and only activates this profile in the later deploy command. These two modules are therefore absent from the version-update reactor and retain their explicit 8.0-SNAPSHOT parent and artifact versions; once the root POM has the tag version, activating them for deploy produces a parent-version mismatch or publishes under snapshot coordinates instead of the tag. Run the version update with this profile active or explicitly update these two POMs before deployment.
Useful? React with 👍 / 👎.
| mv.visitJumpInsn(IFNE, ok); | ||
| invoke(INVOKESTATIC, Names.INTEROP, "divideByZero", "()V", false); |
There was a problem hiding this comment.
The generated check covers only a zero divisor. For int.MinValue / -1 and long.MinValue / -1, CIL signed division must throw OverflowException, while JVM IDIV/LDIV silently returns the minimum value. Translated C# therefore continues with a corrupt arithmetic result and cannot catch the expected exception; add the minimum-value/negative-one check before emitting the division opcode.
Useful? React with 👍 / 👎.
| public static int Abs(int v) { | ||
| return v < 0 ? -v : v; |
There was a problem hiding this comment.
System.Math.Abs(int.MinValue) must throw OverflowException, because the positive magnitude is not representable, but negating it here wraps and returns the same negative value; the long overload has the same defect. Scripts using the documented System.Math surface can therefore propagate a negative value from an operation guaranteed to be nonnegative. Detect the minimum values and throw the runtime's OverflowException before negating.
Useful? React with 👍 / 👎.
… perf baseline On the Ubuntu runner dotnet is linked into /usr/bin, so dropping every PATH directory that holds it also dropped env, bash and mvn. The directory is now shadowed by links to everything in it except dotnet. objectAllocation RAM on Neoverse-N2 is bimodal; the row is recalibrated from this run and three master runs with a wider RAM tolerance. No improvement is claimed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Review findings (rationale for the ones left alone is in comments at the code concerned): - Install the view's services (PlayerPrefs store, audio, platform) before the first scene wakes. - A coroutine started from Start or Update resumes its first null yield on the next frame. - Time.timeSinceLevelLoad restarts on a scene load. - Application.platform and isMobilePlatform come from the host platform. - Signed division and remainder of MinValue by -1 throw OverflowException, as does System.Math.Abs of MinValue. JavaScript screenshots: XYChart rotated the canvas and rotated it back, which with float-rounded angles leaves a transform 1e-14 off identity, and the JavaScript port draws DOM text only for an exact identity. XYChart and ChartComponent now restore the transform they were handed, and the port's DOM text decision tolerates that residue. A differential fixture against the JVM found four older JavaScript target bugs: int to float/double read an un-normalized int, float/double to int wrapped instead of saturating, a static read before its first write was undefined, and Float.toString printed the double. The offscreen check now runs on every scene sample in the integration test. It found: a flick release delivered at the press point because CodenameOneImplementation aliased the shared pointer arrays, GameView missing single finger presses after the first drag, and a sub-image cache keyed by corner only. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
…e numeric fixture The JavaScript target now truncates a float product the way the JVM does, so the six theme screens that size from one move by a pixel; the goldens are this run's CI captures. JsNumericDifferentialApp compares narrowing compound assignments against the JVM on purpose and joins the other compiler fixtures CodeQL does not read. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Compared 214 screenshots: 214 matched. |
…lLoad Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Compared 171 screenshots: 171 matched. |
|
Compared 175 screenshots: 175 matched. Benchmark Results
Detailed Performance Metrics
|
|
Compared 236 screenshots: 236 matched. |
|
@codex review |
|
Compared 176 screenshots: 176 matched. Benchmark Results
Build and Run Timing
Detailed Performance Metrics
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8a51fcb4b1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| public static boolean Any(IEnumerable_1 source, Class t) { | ||
| return walk(source).MoveNext(); |
There was a problem hiding this comment.
When the source is a custom enumerator or a compiler-generated iterator with a finally block, terminal operations that stop early—such as this Any, First, Contains, ElementAt, and Take—abandon the enumerator without calling Dispose(), even though IEnumerator_1 implements IDisposable. The iterator's cleanup therefore never runs after an early result or predicate exception; wrap enumeration in try/finally and dispose the enumerator on every exit path.
Useful? React with 👍 / 👎.
| public static Object GetValues(Type enumType) { | ||
| Type t = enumType(enumType); | ||
| int[] out = new int[t.values.length]; | ||
| for (int i = 0; i < out.length; i++) { | ||
| out[i] = (int) t.values[i]; |
There was a problem hiding this comment.
For any enum whose underlying type is not int—for example, enum E : long { Huge = 1L << 40 }—Enum.GetValues always returns an int[] and truncates every member. Translated callers cast the result to the enum's normalized JVM array type (for this example, long[]), so the usual (E[])Enum.GetValues(typeof(E)) either fails with a cast error or receives corrupted values. Construct the primitive array corresponding to the enum's recorded underlying type instead.
Useful? React with 👍 / 👎.
| for (int i = 1; i < out.length; i++) { | ||
| int v = out[i]; | ||
| int j = i - 1; | ||
| while (j >= 0 && out[j] > v) { | ||
| out[j + 1] = out[j]; |
There was a problem hiding this comment.
When an enum contains negative members, such as the common flags value All = -1, this signed comparison places that member before zero. Enum.GetValues orders members by the unsigned magnitude of their binary representation, so { All = -1, None = 0, One = 1 } must enumerate as None, One, All, whereas this code returns All, None, One. Use an unsigned comparison appropriate to the enum's underlying width.
Useful? React with 👍 / 👎.
| } else if (c == '}') { | ||
| if (i + 1 < n && format.charAt(i + 1) == '}') { | ||
| i++; | ||
| } | ||
| sb.append('}'); |
There was a problem hiding this comment.
For a malformed composite format such as "value }", .NET throws FormatException, but this branch appends the unmatched brace and returns normally. This changes control flow for scripts that validate or recover from user-provided format strings; only a doubled }} should emit a literal closing brace, while a standalone } must be rejected.
Useful? React with 👍 / 👎.
| public static Type $of(Class type, String fullName) { | ||
| Type t = (Type) byClass.get(type); | ||
| if (t == null) { | ||
| t = new Type(fullName, type, null, null, false); | ||
| byClass.put(type, t); |
There was a problem hiding this comment.
The cache key is only the JVM Class, but multiple distinct .NET primitives share one box class: int and uint both use Integer.class, long and ulong use Long.class, and byte and sbyte use Byte.class. Consequently, whichever type is requested first is returned for both, making typeof(int) == typeof(uint) true and potentially reporting the wrong FullName. Include the .NET type identity in the cache key rather than keying solely by the erased Java class.
Useful? React with 👍 / 👎.
| static long number(Object value) { | ||
| if (value instanceof EnumBox) { | ||
| return ((EnumBox) value).value; |
There was a problem hiding this comment.
This conversion discards the EnumBox.type, so APIs such as Enum.IsDefined, Enum.GetName, HasFlag, and CompareTo accept a boxed member of an unrelated enum and operate only on its numeric value. For example, firstEnum.HasFlag(secondEnum) can return true when their bits overlap, whereas .NET throws ArgumentException for differing enum types. Validate the box's enum type against the requested or receiver type before extracting its number.
Useful? React with 👍 / 👎.
| public void set_time(float value) { | ||
| startedAt = Time.unscaledTime - value; | ||
| } |
There was a problem hiding this comment.
When a playing source executes audio.time = value, this setter only changes the runtime's bookkeeping and never seeks the device's Media, so the audible clip continues from its old position while scripts observe a different position. The bookkeeping is also immediately wrong when pitch != 1, because get_time() multiplies elapsed time by pitch but this setter does not divide the requested clip position by pitch. Add a seek operation to AudioOutput and derive startedAt consistently with the playback rate.
Useful? React with 👍 / 👎.
| public static String Trim(String s) { | ||
| return s.trim(); | ||
| } |
There was a problem hiding this comment.
String.trim() only removes characters at or below U+0020, whereas System.String.Trim() removes Unicode whitespace. Inputs surrounded by characters such as non-breaking space (U+00A0) or em space (U+2003) therefore remain untrimmed, even though this runtime already recognizes them in Char_.IsWhiteSpace. Scan the ends using the .NET whitespace predicate instead of delegating to Java's trim().
Useful? React with 👍 / 👎.
| String wanted = value.trim(); | ||
| for (int i = 0; i < t.names.length; i++) { | ||
| if (ignoreCase ? t.names[i].equalsIgnoreCase(wanted) : t.names[i].equals(wanted)) { | ||
| return new EnumBox(t, t.values[i]); | ||
| } | ||
| } | ||
| try { | ||
| return new EnumBox(t, Long.parseLong(wanted)); |
There was a problem hiding this comment.
Enum.Parse accepts a comma-separated list of member names, which is the standard way to parse flags such as "Read, Write"; this implementation only checks the entire string against one member and then attempts numeric parsing. Valid combined values therefore throw ArgumentException. Split on commas, trim each name, resolve each with the requested case sensitivity, and combine their underlying values.
Useful? React with 👍 / 👎.
| if (t instanceof NegativeArraySizeException) { | ||
| return new OverflowException(); | ||
| } | ||
| return t; |
There was a problem hiding this comment.
Several public runtime APIs throw java.lang.IllegalArgumentException for invalid C# calls, including PlayerPrefs, SceneManager, Input, physics queries, and Instantiate, but this dispatcher leaves that throwable unchanged. Because generated catch clauses test against the custom System.Exception hierarchy, even catch (Exception) cannot catch these failures and the raw JVM exception escapes the script. Throw the corresponding compatibility exception at each API or map Java argument exceptions here.
Useful? React with 👍 / 👎.
|
Compared 176 screenshots: 176 matched. Benchmark Results
Build and Run Timing
Detailed Performance Metrics
|
…seline Base library: LINQ operators dispose their enumerators on every exit, Enum values keep their underlying type and unsigned order, Enum.Parse takes comma lists, HasFlag/CompareTo/IsDefined refuse another enum's member, String.Format rejects a lone closing brace, Trim follows Char.IsWhiteSpace, typeof(int) and typeof(uint) are distinct, a float key's two zeros are one key, and a Java exception thrown by the runtime is caught by a script's catch (Exception). AudioSource.time seeks the device's media and accounts for pitch. The macOS draw-gradient golden is this branch's CI capture: with -ffp-contract=off the mutable-image ellipse edge now equals the on-screen one. Linux x64 perf baseline rows recalibrated per runner CPU from PR and master runs; the shift is present on master at the base commit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f0abf10218
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| Image image = (Image) paintedTexts.get(key); | ||
| if (image == null) { | ||
| image = paint(d, width, height); | ||
| paintedTexts.put(key, image); |
There was a problem hiding this comment.
When a Text component displays a changing value such as a timer, score, or frame counter, every distinct string creates an Image and stores it permanently in paintedTexts; the map is never cleared or evicted. A frequently updated label therefore retains thousands of raster images and can exhaust device memory during an otherwise normal game session. Keep only current/recent entries or use a bounded cache.
Useful? React with 👍 / 👎.
| if (yielded instanceof WaitForSeconds) { | ||
| c.resumeAt = Time.time + ((WaitForSeconds) yielded).seconds; | ||
| } else if (yielded instanceof Coroutine) { | ||
| c.waitingFor = (Coroutine) yielded; | ||
| } else if (yielded instanceof WaitForFixedUpdate) { |
There was a problem hiding this comment.
When a coroutine uses the common yield return ChildRoutine() form, the yielded value is an IEnumerator, but this chain recognizes only a started Coroutine; it consequently treats the child iterator like null, never executes it, and resumes the parent on the next frame. Add nested-enumerator handling so the parent waits while the yielded iterator is driven to completion.
Useful? React with 👍 / 👎.
| public static float get_unscaledDeltaTime() { | ||
| return unscaledDeltaTime; |
There was a problem hiding this comment.
When Time.unscaledDeltaTime is read from FixedUpdate, Unity reports the unscaled duration of that fixed step, but this getter always returns the enclosing render frame's dt. At nonmatching render/physics rates—or when several fixed steps run in one frame—fixed-update timers and movement therefore advance by the render delta once per physics step, producing speed and scheduling errors.
Useful? React with 👍 / 👎.
| if (f.isDirectory()) { | ||
| collectStamped(f, prefix + f.getName() + "/", out); | ||
| } else { | ||
| out.add(prefix + f.getName() + ":" + f.length() + ":" + f.lastModified()); |
There was a problem hiding this comment.
In the new Unity incremental build cache, a source or asset rewritten with the same byte length and preserved modification time—such as after restoring or synchronizing files with timestamps intact—produces the same stamp even if its contents changed. run() then accepts state.txt and skips compilation, leaving stale translated classes or resources in the application; hash the file bytes rather than only name, size, and timestamp.
Useful? React with 👍 / 👎.
| public List_1() { | ||
| items = new Object[4]; | ||
| } | ||
|
|
||
| public List_1(int capacity) { | ||
| items = new Object[capacity < 4 ? 4 : capacity]; |
There was a problem hiding this comment.
When valid C# uses new List<T>(source)—including the new List<int>(numbers) array case explicitly anticipated by ArrayView—the translator emits a call to the erased List_1(IEnumerable_1) constructor, but this runtime class exposes only the parameterless and capacity constructors. The Unity project therefore fails translation with a missing runtime method instead of constructing the list; add the enumerable constructor and dispose its enumerator after copying.
Useful? React with 👍 / 👎.
| String simple = meta.getName().substring(0, meta.getName().length() - ".cs.meta".length()); | ||
| for (CilAssembly assembly : scripts) { | ||
| for (TypeDef t : assembly.types()) { | ||
| String full = t.fullName(); | ||
| if (full.equals(simple) || full.endsWith("." + simple)) { | ||
| scriptsByGuid.put(guid, t); |
There was a problem hiding this comment.
When a valid Unity project contains two scripts with the same filename in different folders and namespaces, each .cs.meta GUID is mapped by simple class name and this loop always selects the first matching TypeDef. Both GUIDs can therefore resolve to the same script class, causing scene objects to receive the wrong behaviour and serialized fields from the other script to be dropped. Use source/PDB information or another unambiguous file-to-type mapping.
Useful? React with 👍 / 👎.
| GameObject go = (GameObject) objects.get(i); | ||
| if (go.destroyed || !go.activeInHierarchy()) { | ||
| continue; | ||
| } | ||
| if ((name != null && !go.name.equals(name)) || (tag != null && !go.tag.equals(tag))) { | ||
| continue; |
There was a problem hiding this comment.
When a script calls GameObject.Find("Canvas/Panel/Button"), Unity interprets the slash-separated argument as a hierarchy path, but this implementation compares the whole argument only against each object's leaf name. Such lookups always return null unless an object literally contains slashes in its name, breaking a standard way scripts locate nested scene objects; resolve the root and traverse children as the existing Transform.Find implementation does.
Useful? React with 👍 / 👎.
| public java.lang.Object GetComponentInChildren(Class type) { | ||
| if (!active) { | ||
| return null; | ||
| } | ||
| java.lang.Object c = GetComponent(type); |
There was a problem hiding this comment.
When GetComponentInChildren is invoked on an inactive GameObject, Unity still searches that root object even though the one-argument overload excludes its inactive descendants, but this early return skips the root as well. Managers querying an inactive prefab or temporarily disabled object therefore receive null despite the requested component being attached directly to it; GetComponentsInChildren has the same issue through collect, so both traversals should always inspect their starting object.
Useful? React with 👍 / 👎.
| public static boolean IsDigit(char c) { | ||
| return c >= '0' && c <= '9'; | ||
| } |
There was a problem hiding this comment.
For decimal digits outside ASCII, such as Arabic-Indic ٣ or full-width ５, System.Char.IsDigit must return true because they have the Unicode decimal-digit category, but this implementation accepts only 0 through 9. Translated validation and parsing code therefore rejects digit strings that work in .NET; use a Unicode digit-category check rather than an ASCII range.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 925e929975
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| wsy = (a - b) / len; | ||
| wcos = m00 / len; | ||
| wsin = m10 / len; | ||
| wrot = (float) (com.codename1.unitycompat.system.Math.Atan2(m10, m00) * RAD2DEG); |
There was a problem hiding this comment.
When a parent has non-uniform scale and a child has local rotation, deriving wrot from the composed matrix column makes scale alter the reported world rotation. For example, parent scale (2,1) and child rotation 45° produce atan2(0.707, 1.414) = 26.565°, although the child's Unity world rotation remains 45°. Consequently rotation, eulerAngles, right, up, and TransformDirection all return incorrect values in this common hierarchy; retain the quaternion/angle composition separately from the affine basis used by TransformPoint.
Useful? React with 👍 / 👎.
| float min = minNormalAngle < maxNormalAngle ? minNormalAngle : maxNormalAngle; | ||
| float max = minNormalAngle < maxNormalAngle ? maxNormalAngle : minNormalAngle; |
There was a problem hiding this comment.
When a filter uses infinity for a normal-angle bound, this comparison leaves the infinity literal in the range instead of applying Unity's documented sentinel behavior: either sign of infinity means the lowest possible value for minNormalAngle and the highest possible value for maxNormalAngle (minimum, maximum). For example, SetNormalAngle(+Infinity, +Infinity) should admit the full angular range, but here every finite angle is below min and is filtered out; normalize each bound according to whether it is the minimum or maximum before testing.
Useful? React with 👍 / 👎.
| sb.append(sb.length() == 0 ? "" : ", ").append(real(key.get("time"), "0")).append(", ") | ||
| .append(real(value, "0")).append(", ").append(real(in, "0")).append(", ") | ||
| .append(real(out, "0")); |
There was a problem hiding this comment.
When an animation key uses weightedMode In, Out, or Both with a non-default inWeight or outWeight, this emits only its time, value, and slopes, so AnimationClip.sample evaluates it as an ordinary unweighted Hermite segment. Unity uses those weights when calculating the affected curve segments (WeightedMode documentation), so importing such a clip silently changes its animated positions, scales, colors, or other properties; serialize and evaluate the mode and weights rather than discarding them.
Useful? React with 👍 / 👎.
| } else if ("2".equals(mode)) { | ||
| passed = nodes.get(fileId(arguments.get("m_ObjectArgument"))); |
There was a problem hiding this comment.
When an Inspector-configured persistent listener passes an asset such as an AudioClip, sprite, or prefab through its object argument, m_ObjectArgument contains an external GUID/file ID rather than a node in the current scene graph. Looking it up only in nodes therefore produces null, and the generated listener passes null even though these asset types are supported elsewhere by the compiler; resolve this reference through the normal asset lookup path before emitting the call.
Useful? React with 👍 / 👎.
| if (one) { | ||
| CilType param = types.storage(m.sig.params[0]); | ||
| if (wanted != null ? param.kind != wanted : types.isStruct(param) | ||
| || universe.definitionOf(param) == null) { | ||
| continue; |
There was a problem hiding this comment.
When a persistent listener targets overloaded public methods that each accept a different reference type, this accepts whichever one appears first in metadata without consulting m_ObjectArgumentAssemblyTypeName or checking the serialized argument's assignability. For example, if Handle(AudioClip) precedes the Inspector-selected Handle(GameObject), the compiler binds the former and later substitutes null for the incompatible GameObject, invoking the wrong overload; use the recorded argument type to select the method before generating the call.
Useful? React with 👍 / 👎.
…rmal-angle bounds, persistent calls pick the recorded overload and resolve asset arguments; weighted-tangent build warning Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2c6481936b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (key.startsWith("m_") || "serializedVersion".equals(key)) { | ||
| continue; |
There was a problem hiding this comment.
When a script declares a serialized field with the common m_ prefix, such as [SerializeField] private int m_health, Unity writes that exact key into the scene or prefab YAML, but this branch discards it before attempting findField. The generated scene therefore leaves the field at its initializer/default value without warning; skip only the known MonoBehaviour metadata keys, or try resolving a serialized script field first.
Useful? React with 👍 / 👎.
| line(sb, v + ".$setup(" + f(offset.get("x"), "0") + ", " + f(offset.get("y"), "0") + ", " | ||
| + f(offset.get("z"), "-10") + ", " + f(p.get("m_XDamping"), "1") + ", " | ||
| + f(p.get("m_YDamping"), "1") + ");"); |
There was a problem hiding this comment.
When a scene uses CinemachineTransposer with a target-relative m_BindingMode such as LockToTarget, this setup serializes only the offset and damping, so the runtime always adds the raw offset in world axes. Rotating the Follow target therefore leaves the camera on the same world side instead of rotating it around the target, contrary to the documented binding-mode behavior; pass the serialized mode through and interpret the offset in its selected coordinate space.
Useful? React with 👍 / 👎.
| int n = UnityRuntime.$sceneCount(); | ||
| for (int i = 0; i < n; i++) { | ||
| String path = UnityRuntime.$scenePath(i); | ||
| if (path.equals(sceneName) || name(path).equals(sceneName)) { |
There was a problem hiding this comment.
When the build contains Assets/Scenes/Level.unity, calls such as LoadScene("Assets/Scenes/Level") or LoadScene("level") fail these exact comparisons and throw, even though Unity accepts a full build-settings path without the .unity extension and compares scene names case-insensitively (Unity documentation). Strip the extension from the stored path and compare both supported forms without regard to case.
Useful? React with 👍 / 👎.
…ing mode turns the offset with the target, LoadScene accepts extensionless paths and any case Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2a174f00f5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| UnityPomUpdater(String pom) { | ||
| String p = pom; | ||
| if (p.indexOf("<artifactId>codenameone-unity-compat</artifactId>") < 0) { |
There was a problem hiding this comment.
When a legacy common POM already mentions codenameone-unity-compat only in <dependencyManagement> (or even in a comment), this substring check treats the runtime as installed and skips adding the active profile. The import then reports success, but CompileUnityMojo cannot find the runtime in project.getArtifacts(), so the imported Unity project fails to build. Parse the dependency sections and require an effective dependency rather than any textual occurrence.
Useful? React with 👍 / 👎.
| if (output != null) { | ||
| output.play(v, c, level(volumeScale), pitch, false); |
There was a problem hiding this comment.
When a script calls PlayOneShot(clip, -1f), this passes a negative gain to AudioOutput and saves it for later volume updates, even though that interface requires a 0–1 volume. Unity automatically clamps negative volumeScale values to zero, so the call should produce silence rather than backend-dependent behavior such as an invalid or inverted gain; clamp the scale before playing and retaining it. Unity AudioSource.PlayOneShot documentation
Useful? React with 👍 / 👎.
| for (Object removed : list(modification.get("m_RemovedComponents"))) { | ||
| source.removeAll(targets(source, removed)); | ||
| } |
There was a problem hiding this comment.
When a prefab instance has a nonempty m_RemovedGameObjects list, this expansion applies removed components but never removes the listed GameObjects, so the compiled scene retains the deleted child together with its renderers, colliders, and scripts. Unity defines this list as the source-prefab GameObjects removed from this particular instance; resolve each entry and remove that object's whole subtree from source. Unity YAML prefab serialization documentation
Useful? React with 👍 / 👎.
| if (c == '\\' && i + 1 < s.length()) { | ||
| char n = s.charAt(++i); | ||
| int digits = n == 'u' ? 4 : n == 'x' ? 2 : 0; | ||
| if (digits > 0 && i + digits < s.length()) { | ||
| try { | ||
| sb.append((char) Integer.parseInt(s.substring(i + 1, i + 1 + digits), 16)); | ||
| i += digits; | ||
| continue; | ||
| } catch (NumberFormatException e) { | ||
| // Not an escape after all; keep the letter. | ||
| } | ||
| } | ||
| sb.append(n == 'n' ? '\n' : n == 't' ? '\t' : n == 'r' ? '\r' : n); |
There was a problem hiding this comment.
When a serialized string contains a YAML escape other than n, t, r, x, or u, this fallback silently drops the backslash and keeps the escape letter; for example, "a\bb" becomes "abb" instead of containing a backspace, and \U0001F600 becomes literal text. UnityYAML supports double-quoted scalars, and YAML 1.1 requires escapes such as \0, \a, \b, \v, \f, \e, and eight-digit \U to decode to their Unicode values, so serialized script strings are otherwise corrupted during compilation. UnityYAML format, YAML 1.1 escape sequences
Useful? React with 👍 / 👎.
…very YAML escape decodes, negative one-shot volume is silence; the importer ignores a runtime named only in comments or dependency management Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7379c3b8f0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return (f.flags & FIELD_ACCESS_MASK) == FIELD_PUBLIC | ||
| || type.assembly.attributes(f.token()).contains("UnityEngine.SerializeField"); |
There was a problem hiding this comment.
When a public or [SerializeField] field has an unsupported type such as int[][], this predicate still returns true. Fresh evidence after the earlier serialization-rule fix is that only field modifiers are checked, even though Unity does not serialize jagged arrays, dictionaries, or nested containers (Unity serialization rules); emitSerializedCopies consequently clones any SZARRAY and shallow-copies nested arrays during Instantiate, causing clones to copy and share state that Unity would leave uninitialized. Include recursive field-type eligibility in this predicate.
Useful? React with 👍 / 👎.
…ponent walks pointer listeners by index (ComponentTest ConcurrentModificationException) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bbacea63c6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| File stale = new File(target, path); | ||
| if (stale.isFile() && stale.delete()) { |
There was a problem hiding this comment.
When a previously imported directory is removed from the source and its destination is replaced with a symlink, a recorded child such as Assets/old/file.txt reaches this cleanup without passing through copy(). isFile() follows the linked parent and delete() then removes the external target file. The new exact-destination link check in copy() therefore does not close this path; validate every stale path's ancestors or resolve and verify containment before deleting.
Useful? React with 👍 / 👎.
| for (int i = 0; i < list.size(); i++) { | ||
| if (!(list.get(i) instanceof NativeTextSelectionListener)) { |
There was a problem hiding this comment.
When another thread removes a listener after list.size() succeeds but before list.get(i), this indexed traversal can still throw IndexOutOfBoundsException; removing an earlier element can also make it skip an application listener. Since EventDispatcher exposes its mutable ArrayList and synchronizes mutations on the dispatcher, take a snapshot or traverse while holding that same lock rather than replacing one unsafe iterator with another unsynchronized traversal.
Useful? React with 👍 / 👎.
…der; Component's listener walk documents why it takes no lock Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7ea5d73e28
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (rest < duration) { | ||
| break; | ||
| } | ||
| rest -= duration; |
There was a problem hiding this comment.
When a looping particle system has a tiny positive duration such as 1e-20f, a normal frame leaves rest around 0.016f, and rest -= duration rounds back to exactly the same float. The loop condition therefore remains true forever and blocks the frame thread. Use a direct remainder calculation or apply the same no-progress guard already used in the emission-segment loop above.
Useful? React with 👍 / 👎.
| if (v > 709.78) { | ||
| return 1.0 / 0.0; |
There was a problem hiding this comment.
When translated code calls Math.Exp with an input between 709.78 and the actual overflow boundary, such as 709.781, this branch returns positive infinity even though the correct result is still finite (approximately 1.7946e308). This also affects non-integral Math.Pow calls that delegate to Exp; use the true boundary near 709.782712893384 or let the scaling calculation determine whether the result overflows.
Useful? React with 👍 / 👎.
| // A record is only ever written by install(), but it is a file | ||
| // in the build directory: a line that climbs out of the classes | ||
| // directory is not followed. | ||
| if (rel.length() > 0 && !rel.contains("..")) { |
There was a problem hiding this comment.
When a used Unity image has a legal name such as icon..png and the Maven module later removes src/main/unity, its installed path contains .. but no parent-directory component. This condition skips it, so removeOutput() deletes the record and work tree while leaving the image in target/classes, causing subsequent packages to continue shipping a stale asset with no record left to remove it. Reject exact .. path components rather than every occurrence of the substring.
Useful? React with 👍 / 👎.
|
|
||
| public void StopCoroutine(Coroutine routine) { | ||
| if (routine != null) { | ||
| routine.done = true; |
There was a problem hiding this comment.
When one script obtains a Coroutine started by another behaviour and calls StopCoroutine on its own instance, this assignment stops the other behaviour's coroutine because it never checks routine.owner. Unity's StopCoroutine contract only stops a coroutine running on the receiving behaviour, so cross-component coroutine handles can unexpectedly cancel unrelated work; ignore or reject handles whose owner is not this.
Useful? React with 👍 / 👎.
…th.Exp keeps finite results to the true boundary, cleanup records reject only a .. component, StopCoroutine stops only its own coroutines Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 98f5be03aa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!installed.contains(rel)) { | ||
| deleteQuietly(new File(classesOut, rel)); |
There was a problem hiding this comment.
When target/unity/installed.txt is edited or restored from a poisoned build cache with an entry such as ../../pom.xml, this stale-output loop resolves the entry relative to classesOut and deletes the file outside the classes directory. removeOutput() validates the same record with climbs(rel), but this install path does not; reject parent components or verify canonical containment before deleting recorded paths.
Useful? React with 👍 / 👎.
…ectory are ignored when reinstalling Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
…t resolve instead of failing the Hugo build; the gate's check tests run on a throwaway tree, not the repository's overlays Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
AndroidGradleBuilder overrides unzip, so the prune in Executor.unzip never ran on Android: every plugin pack reached the permission scan, and FirebaseMessaging's com/codename1/push reference switched FCM on, failing the gallery build on a missing google-services.json. Drop pr/6055.json: the latest run's JavaScript and Linux sizes are within tolerance of the current rows (JavaScript once #5976's overlay is re-based, on branch fix-5976-flutter-baseline), so nothing needs rebaselining. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
#5976 merged after the nightly fold had already moved javascript gallery/code_bytes to 13,304,997, so its overlay's 'from' (13,238,731) no longer matched and every baseline read failed: master's Performance baselines check and every PR's Flutter benchmark gate. Its measured value (13,338,532) is unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 2d83e24)
2 participants
Summary
A Unity 2D project (C# scripts, scenes, prefabs, sprites, project settings) dropped into
common/src/main/unity, or imported withcn1:import-unity-project, builds unmodified as a Codename One application for every target. No Unity player and no .NET runtime ship with the app.maven/cil-translator: reads the assembly the C# compiler produced and writes one class file per C# type; compiles scenes, prefabs, animator controllers, clips, tile maps and particle systems to reflection-free Java (UnityAppImpl).maven/unity-compat: the runtime. A .NET base library in Java, theUnityEngineAPI overcom.codename1.gaming(2D rendering, Box2D physics with exact casts and overlap queries, Animator, tile maps, particles, uGUI, touch, audio,PlayerPrefs,Resources), plus our own implementations of the documented TextMesh Pro and Cinemachine API.UnityProjectBuilderinbuild-engine, thecompile-unityandimport-unity-projectMaven goals, the GradlecompileUnitytask, the archetype profile, and release publishing behind-Dunity-compat(the modules need the .NET SDK; the framework build does not).UnityApplication(createForm,onProjectInstalled,onStarted,callInFrame), so a game sits inside a Codename One form and uses the rest of the framework.Unity-Interop.asciidoc.Clean room: no Unity source, assemblies or package source were read or shipped; the stub
UnityEngine.dllis compiled from our own sources.Changes outside the new modules
Bugs this work uncovered, fixed at the root, each with a regression test:
POP2/DUP2/DUP_X2),MULTIANEWARRAYdimension pops, a call result compared viaFCMP/DCMPevaluated more than once,SWAPlosing the lower value.arraycopy/clone of an array of primitive arrays took the primitive path.GameView: a key repeat released a held key after 800 ms; a focused hosted child kept the multi-touch arrays from the view.World.popContact; the broad phase allocated a merge buffer every step above 31 pairs (now an in-place sort, checked againstArrays.sortover randomized pair sets).Behavior changes that affect every application:
-ffp-contract=off.floatresult withMath.fround.Both make float arithmetic match the JVM bit for bit. Their performance cost on ordinary applications has not been measured.
Verification
scripts/unity-compat-samples) produce byte-identical traces on the JVM, ParparVM native and JavaScript (maven/integration-tests/unity-compat-test.sh --parparvm).cil-translatorandunity-compat: SpotBugs 0, Checkstyle 0, PMD forbidden list clean, cast-semantics clean.MapViewMarkerLabelTesttimeout on a loaded machine; passes alone).Not verified
.github/workflows/unity-compat.ymland therelease.ymlchanges have never run on CI.GameInputstill reads edge flags that a press landing mid-frame can miss on ports that render off the EDT; left for a separate decision.🤖 Generated with Claude Code