Add abstract iOS launcher#2829
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new abstract base class IosApplicationLauncher to centralize and simplify iOS application lifecycle management, and refactors IosTestChooserLauncher to extend it. The review feedback highlights a potential issue in IosApplicationLauncher where app.getContext() could return null during startup or shutdown, leading to an unexpected IllegalStateException in both the update() and resize() methods. Adding null checks for the context in these methods is recommended to prevent crashes.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a new base class IosApplicationLauncher in the jme3-ios module to standardize iOS application launching in jMonkeyEngine, and refactors IosTestChooserLauncher to extend this new base class. Feedback on the changes suggests declaring the app field as volatile to ensure thread safety across the launcher and render threads. Additionally, it is recommended to rename the resize method to reshape to preserve backward compatibility, and to avoid throwing IllegalStateException in both update and reshape when the context is not an IGLESContext to prevent application crashes in non-standard or test environments.
| public void resize(int width, int height) { | ||
| if (app == null) return; | ||
| JmeContext context = app.getContext(); | ||
| if (context == null) return; | ||
| if (context instanceof IGLESContext) { | ||
| ((IGLESContext) context).resizeFramebuffer(width, height); | ||
| } else { | ||
| throw new IllegalStateException("Application context is not an IGLESContext"); | ||
| } | ||
| } |
This comment was marked as low quality.
This comment was marked as low quality.
Sorry, something went wrong.
| */ | ||
| public abstract class IosApplicationLauncher { | ||
|
|
||
| protected Application app; |
This comment was marked as low quality.
This comment was marked as low quality.
Sorry, something went wrong.
| public void update() { | ||
| if (app == null) return; | ||
| JmeContext context = app.getContext(); | ||
| if (context == null) return; | ||
| if (context instanceof IGLESContext) { | ||
| ((IGLESContext) context).runFrame(); | ||
| } else { | ||
| throw new IllegalStateException("Application context is not an IGLESContext"); | ||
| } | ||
| } |
This comment was marked as low quality.
This comment was marked as low quality.
Sorry, something went wrong.
1 participant
mimics the AndroidHarnessFragment to make iOS bootstrap easier