fix: add initialization guard to prevent double plugin init#25
Conversation
When the plugin is both enabled in ProjectSettings > Plugins AND the godot_iap.gd script is attached as an AutoLoad node, _ready() runs twice causing duplicate initialization, listeners, and state conflicts. This adds: - _is_initialized guard in _ready() to prevent double native plugin init - _is_connected guard in init_connection() to make it idempotent Closes #24
📝 WalkthroughWalkthroughThe changes add initialization guards to prevent duplicate plugin setup on game startup. A new Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Code Review
This pull request introduces initialization and connection guards to the godot_iap.gd script to prevent redundant setup and ensure idempotency. Specifically, it adds an _is_initialized flag in _ready() and checks the _is_connected state in init_connection(). The review feedback suggests making these state variables static to ensure consistency across multiple instances and using queue_free() to properly handle and remove duplicate instances.
| # Native plugin reference | ||
| var _native_plugin: Object = null | ||
| var _is_connected: bool = false | ||
| var _is_initialized: bool = false |
There was a problem hiding this comment.
To effectively prevent double initialization across multiple instances (e.g., when the script is added as an AutoLoad both by the plugin and manually), this variable should be declared as static.
Note that for the state to be truly shared and consistent across instances, _native_plugin, _is_connected, and _platform should also be made static. Otherwise, a second instance will not have access to the native handle initialized by the first one.
static var _is_initialized: bool = false
| if _is_initialized: | ||
| return | ||
| _is_initialized = true |
There was a problem hiding this comment.
Using a simple return here is problematic because it leaves the second instance in the scene tree but in an uninitialized state. Since variables like _native_plugin and _platform are instance-specific in the current code, the second instance will have them as null/empty. Any subsequent calls to this instance (e.g., init_connection()) will then unexpectedly fall back to "mock mode" even on real devices.
A more robust approach for a singleton Node is to remove the duplicate instance entirely to prevent it from being used.
if _is_initialized:
queue_free()
return
_is_initialized = true
| if _is_connected: | ||
| print("[GodotIap] Already connected, skipping init_connection") | ||
| return true |
There was a problem hiding this comment.
This guard makes init_connection idempotent for a single instance. However, because _is_connected is an instance variable, if multiple instances of this script exist, each one will still attempt to initialize the native connection independently. If the native plugin is a singleton (as it is on Android), this results in redundant calls to the native side. Consider making _is_connected a static variable to ensure idempotency across the entire application state.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@addons/godot-iap/godot_iap.gd`:
- Around line 184-186: The early return guarded by _is_connected in
init_connection can prevent the native initConnection() call from ever running;
modify init_connection so the native initConnection() path always executes on
first run (or add a separate flag, e.g., _native_initialized) before
short-circuiting: either move the _is_connected check to after the native
initConnection() call or change the condition to if _is_connected and
_native_initialized then return true, and set _native_initialized only after a
successful native initConnection() so mock initialization cannot block native
setup (refer to init_connection, initConnection, _is_connected and set a new
_native_initialized or similar flag).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4a60eb4d-e5d2-4c28-a875-f9ad5dbfc1fd
📒 Files selected for processing (1)
addons/godot-iap/godot_iap.gd
| if _is_connected: | ||
| print("[GodotIap] Already connected, skipping init_connection") | ||
| return true |
There was a problem hiding this comment.
_is_connected short-circuit can skip the first native initConnection() call.
At Lines 184-186, if init_connection() runs before native setup, the function can hit mock path, set _is_connected = true, and later always return early—never invoking native initConnection(). This is high-impact because Example/iap_manager.gd initializes once and caches the result.
Proposed fix
func init_connection() -> bool:
print("[GodotIap] init_connection called")
+ if not _is_initialized:
+ _platform = OS.get_name()
+ _init_native_plugin()
+ _is_initialized = true
if _is_connected:
print("[GodotIap] Already connected, skipping init_connection")
return true📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if _is_connected: | |
| print("[GodotIap] Already connected, skipping init_connection") | |
| return true | |
| func init_connection() -> bool: | |
| print("[GodotIap] init_connection called") | |
| if not _is_initialized: | |
| _platform = OS.get_name() | |
| _init_native_plugin() | |
| _is_initialized = true | |
| if _is_connected: | |
| print("[GodotIap] Already connected, skipping init_connection") | |
| return true |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@addons/godot-iap/godot_iap.gd` around lines 184 - 186, The early return
guarded by _is_connected in init_connection can prevent the native
initConnection() call from ever running; modify init_connection so the native
initConnection() path always executes on first run (or add a separate flag,
e.g., _native_initialized) before short-circuiting: either move the
_is_connected check to after the native initConnection() call or change the
condition to if _is_connected and _native_initialized then return true, and set
_native_initialized only after a successful native initConnection() so mock
initialization cannot block native setup (refer to init_connection,
initConnection, _is_connected and set a new _native_initialized or similar
flag).
|
Thank you for this fix! We've applied this change in the OpenIAP monorepo: hyodotdev/openiap#87 Going forward, all godot-iap development happens in the OpenIAP monorepo. This repository will be archived soon — please submit future PRs there. |
## Summary - Add `static var _is_initialized` guard in `_ready()` to prevent double native plugin init across all instances - Always call through to native `init_connection()` to avoid false-positive success on iOS async connections When the plugin is both enabled in ProjectSettings > Plugins AND the `godot_iap.gd` script is attached as an AutoLoad node, `_ready()` runs twice causing duplicate initialization, listeners, and state conflicts. Follows up from hyochan/godot-iap#25 / hyochan/godot-iap#24 ## Test plan - [x] Plugin works correctly with both ProjectSettings and AutoLoad enabled - [x] No duplicate signal connections — verified via `test_no_duplicate_signal_connections` 🤖 Generated with [Claude Code](https://claude.ai/code) --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
When the plugin is both enabled in ProjectSettings > Plugins AND the godot_iap.gd script is attached as an AutoLoad node, _ready() runs twice causing duplicate initialization, listeners, and state conflicts.
This adds:
Closes #24
Summary by CodeRabbit