Skip to content

Comments

Fix LoginManager#1626

Merged
chsami merged 2 commits intochsami:developmentfrom
gmason0:fix/login-manager
Dec 10, 2025
Merged

Fix LoginManager#1626
chsami merged 2 commits intochsami:developmentfrom
gmason0:fix/login-manager

Conversation

@gmason0
Copy link
Contributor

@gmason0 gmason0 commented Dec 10, 2025

I think this was vibed a little too hard...

Issue

the wasLoggedIn variable inside of onGameStateChanged was changed to leverage this new class called LoginManager.java, when this happened, this was specifically targeting if the current GameState is LOGGED_IN, which we know is already true as if we look at MicrobotPlugin.java#L301, we have an if statement that if (gameStateChanged.getGameState() == GameState.LOGGED_IN).. Due to this, this caused Rs2RunePouch.fullUpdate(); & Rs2CacheManager.registerEventHandlers(); to be unreachable.

idea64_OcfFkMCVIZ

Fix

I have created a new member variable inside of the LoginManager to store the previously "known" game state by saving it at the end of the onGameStateChanged event. this way we can compare against it inside of the primary logic above. This will allow this previously unreachable logic to function & fixes the initial load of the Runepouch data.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 10, 2025

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

The changes refactor login state tracking across MicrobotPlugin and LoginManager to use game state comparison instead of a boolean flag. MicrobotPlugin now checks if the last known game state equals GameState.LOGGED_IN rather than calling isLoggedIn(). LoginManager introduces an AtomicReference<GameState> field to store the last known game state, converts lastLoginTimestamp to AtomicReference<Instant> for thread-safe access, and adds public getter/setter methods for game state persistence. The login state is now synchronized via setLastKnownGameState() called during game state changes.

Possibly related PRs

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title check ❓ Inconclusive The title 'Fix LoginManager' is vague and doesn't clearly convey the specific issue being fixed or the main change introduced. Consider a more descriptive title like 'Restore unreachable login state logic using last known game state' to better communicate the fix.
✅ Passed checks (1 passed)
Check name Status Explanation
Description check ✅ Passed The description clearly explains the problem, shows the root cause, includes visual evidence, and describes the solution implemented.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/security/LoginManager.java (1)

44-45: Inconsistent accessor pattern between atomic reference fields.

LAST_KNOWN_GAME_STATE uses explicit getter/setter methods that unwrap the GameState value (lines 54-60), but lastLoginTimestamp uses Lombok's @Getter which exposes the raw AtomicReference<Instant>. This creates an inconsistent API:

  • getLastKnownGameState() → returns GameState
  • getLastLoginTimestamp() → returns AtomicReference<Instant>

Consider either removing the @Getter and adding explicit accessors that return Instant, or documenting that the raw reference is intentionally exposed.

-	@Getter
-	private static AtomicReference<Instant> lastLoginTimestamp = new AtomicReference<>(null);
+	private static final AtomicReference<Instant> lastLoginTimestamp = new AtomicReference<>(null);
+
+	public static Instant getLastLoginTimestamp() {
+		return lastLoginTimestamp.get();
+	}
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3cab689 and 4e4657e.

📒 Files selected for processing (2)
  • runelite-client/src/main/java/net/runelite/client/plugins/microbot/MicrobotPlugin.java (2 hunks)
  • runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/security/LoginManager.java (3 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/**/*.java

📄 CodeRabbit inference engine (AGENTS.md)

runelite-client/src/main/java/net/runelite/client/plugins/microbot/**/*.java: Gameplay automation lives in runelite-client/src/main/java/net/runelite/client/plugins/microbot; keep new scripts and utilities inside this plugin
Register new automation under net.runelite.client.plugins.microbot and reuse the scheduler pattern shown in ExampleScript

Files:

  • runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/security/LoginManager.java
  • runelite-client/src/main/java/net/runelite/client/plugins/microbot/MicrobotPlugin.java
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/**/*.java

📄 CodeRabbit inference engine (AGENTS.md)

Shared helpers sit under .../microbot/util

Files:

  • runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/security/LoginManager.java
**/*.java

📄 CodeRabbit inference engine (AGENTS.md)

**/*.java: Keep indentation with tabs, follow the brace placement already in MicrobotPlugin.java, and prefer lines under 120 characters
Use UpperCamelCase for types, lowerCamelCase for members, and prefix configuration interfaces with the plugin name (e.g., ExampleConfig)
Rely on Lombok for boilerplate where already adopted

Files:

  • runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/security/LoginManager.java
  • runelite-client/src/main/java/net/runelite/client/plugins/microbot/MicrobotPlugin.java
runelite-client/src/main/java/net/runelite/client/plugins/microbot/*Plugin.java

📄 CodeRabbit inference engine (AGENTS.md)

When adding panel controls or overlays, update the Microbot navigation panel setup in MicrobotPlugin and provide default config values

Files:

  • runelite-client/src/main/java/net/runelite/client/plugins/microbot/MicrobotPlugin.java
🧠 Learnings (5)
📚 Learning: 2025-11-24T19:05:32.967Z
Learnt from: CR
Repo: chsami/Microbot PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T19:05:32.967Z
Learning: Applies to runelite-client/src/main/java/net/runelite/client/plugins/microbot/**/*.java : Gameplay automation lives in `runelite-client/src/main/java/net/runelite/client/plugins/microbot`; keep new scripts and utilities inside this plugin

Applied to files:

  • runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/security/LoginManager.java
  • runelite-client/src/main/java/net/runelite/client/plugins/microbot/MicrobotPlugin.java
📚 Learning: 2025-11-24T19:05:32.967Z
Learnt from: CR
Repo: chsami/Microbot PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T19:05:32.967Z
Learning: Applies to runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/**/*.java : Shared helpers sit under `.../microbot/util`

Applied to files:

  • runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/security/LoginManager.java
  • runelite-client/src/main/java/net/runelite/client/plugins/microbot/MicrobotPlugin.java
📚 Learning: 2025-11-24T19:05:32.967Z
Learnt from: CR
Repo: chsami/Microbot PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T19:05:32.967Z
Learning: Applies to runelite-client/src/main/java/net/runelite/client/plugins/microbot/*Plugin.java : When adding panel controls or overlays, update the Microbot navigation panel setup in `MicrobotPlugin` and provide default config values

Applied to files:

  • runelite-client/src/main/java/net/runelite/client/plugins/microbot/MicrobotPlugin.java
📚 Learning: 2025-11-24T19:05:32.967Z
Learnt from: CR
Repo: chsami/Microbot PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T19:05:32.967Z
Learning: Applies to runelite-client/src/main/java/net/runelite/client/plugins/microbot/example/**/*.java : Runnable examples live in `.../microbot/example`

Applied to files:

  • runelite-client/src/main/java/net/runelite/client/plugins/microbot/MicrobotPlugin.java
📚 Learning: 2025-11-24T19:05:32.967Z
Learnt from: CR
Repo: chsami/Microbot PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T19:05:32.967Z
Learning: Applies to runelite-client/src/main/java/net/runelite/client/plugins/microbot/**/*.java : Register new automation under `net.runelite.client.plugins.microbot` and reuse the scheduler pattern shown in `ExampleScript`

Applied to files:

  • runelite-client/src/main/java/net/runelite/client/plugins/microbot/MicrobotPlugin.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Build
🔇 Additional comments (1)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/MicrobotPlugin.java (1)

306-331: Login state tracking logic looks correct.

The flow properly checks wasLoggedIn against the previous state before running the initialization code, then updates setLastKnownGameState at the end of the handler. This ensures Rs2RunePouch.fullUpdate() and Rs2CacheManager.registerEventHandlers() execute only on a fresh login transition, which addresses the original bug.

@chsami chsami merged commit d65cfb4 into chsami:development Dec 10, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants