Rs2Cache automatic integration

[Krulvis]

I was working on a caching system for PoH and noticed how tightly coupled and implicit all of the requirements and implementations are.

The current implementation of Rs2CacheManager, CacheSerializationManager and any Rs2Cache class requires someone to remember all the steps that are necessary for integrating the new Rs2Cache into the manager. There's no compile-time enforcement that checks for "completeness". Some of the difficulties this causes are:

• New caches are not automatically registered and integrated into the manager
• The manager ends up hardcoding knowledge of every cache type, instead of caches declaring their own capabilities.
• Whenever a cache adds features or changes in structure, these are not automatically reflected in the cache management and require manual going back and forth through all classes that are
• Scalability issues. Rs2CacheManager will just continuously grow with every cache type added.
  -...

I'll start by giving some examples:
Rs2CacheManager.java

/**
 * Loads persistent caches for a specific profile.
 *
 * @param profileKey The RuneLite profile key to load caches for
 */
private static void loadPersistentCaches(String profileKey) {
    try {
        if (profileKey == null) {
            log.warn("Cannot load persistent caches: profile key is null");
            return;
        }

        Rs2CacheManager.rsProfileKey = profileKey;

        log.info("Loading persistent caches from configuration for profile: {}", profileKey);

        // Load Skills cache
        if (Rs2SkillCache.getCache().isPersistenceEnabled()) {
            CacheSerializationManager.loadCache(Rs2SkillCache.getCache(), Rs2SkillCache.getCache().getConfigKey(), profileKey,false);
            log.info("Loaded Skills cache from configuration, new cache size: {}",
                      Rs2SkillCache.getCache().size());
        }

        // Load Quest cache  
        if (Rs2QuestCache.getCache().isPersistenceEnabled()) {
            CacheSerializationManager.loadCache(Rs2QuestCache.getCache(), Rs2QuestCache.getCache().getConfigKey(), profileKey,false);
            // Schedule an async update to populate quest states from client without blocking initialization
            //Rs2QuestCache.updateAllFromClientAsync();
            log.debug ("Loaded Quest cache from configuration, new cache size: {}",
                      Rs2QuestCache.getCache().size());
        }

        // Load Varbit cache
        if (Rs2VarbitCache.getCache().isPersistenceEnabled()) {
            CacheSerializationManager.loadCache(Rs2VarbitCache.getCache(), Rs2VarbitCache.getCache().getConfigKey(), profileKey,false);
            log.debug ("Loaded Varbit cache from configuration, new cache size: {}",
                      Rs2VarbitCache.getCache().size());
        }

        // Load VarPlayer cache
        if (Rs2VarPlayerCache.getCache().isPersistenceEnabled()) {
            CacheSerializationManager.loadCache(Rs2VarPlayerCache.getCache(), Rs2VarPlayerCache.getCache().getConfigKey(), profileKey, false);
            log.debug ("Loaded VarPlayer cache from configuration, new cache size: {}",
                      Rs2VarPlayerCache.getCache().size());
        }
        if (Rs2SpiritTreeCache.getCache().isPersistenceEnabled()) {
            CacheSerializationManager.loadCache(Rs2SpiritTreeCache.getCache(), Rs2SpiritTreeCache.getCache().getConfigKey(), profileKey,false);
             // Update spirit tree cache with current farming handler data after initial load
            try {
                Rs2SpiritTreeCache.getInstance().update();
                if(Microbot.isDebug()) Rs2SpiritTreeCache.logState(LogOutputMode.CONSOLE_ONLY);
                log.debug("Spirit tree cache updated from FarmingHandler after initial load");
            } catch (Exception e) {
                log.warn("Failed to update spirit tree cache from FarmingHandler after initial load: {}", e.getMessage());
            }
            log.debug ("Loaded SpiritTree cache from configuration, new cache size: {}",
                      Rs2SpiritTreeCache.getCache().size());
        }

        log.info("Successfully loaded all persistent caches from configuration for profile: {}", profileKey);
    } catch (Exception e) {
        log.error("Failed to load persistent caches from configuration for profile: {}", profileKey, e);
    }
}

CacheSerializationManager.java

private static <K, V> void deserializeCacheData(Rs2Cache<K, V> cache, String configKey, String json) {
  try {
      log.debug("Starting deserialization for cache type: {}, JSON length: {}", configKey, json != null ? json.length() : 0);

      switch (configKey) {
          case "skills":
              deserializeSkillCache((Rs2Cache<Skill, SkillData>) cache, json);
              break;
          case "quests":
              deserializeQuestCache((Rs2Cache<Quest, QuestState>) cache, json);
              break;
          case "varbits":
              deserializeVarbitCache((Rs2Cache<Integer, VarbitData>) cache, json);
              break;
          case "varPlayerCache":
              deserializeVarPlayerCache((Rs2Cache<Integer, VarbitData>) cache, json);
              break;
          case "spiritTrees":
              deserializeSpiritTreeCache((Rs2Cache<SpiritTree, SpiritTreeData>) cache, json);
              break;
          default:
              log.warn("Unknown cache type for deserialization: {}", configKey);
      }

      log.debug("Deserialization completed for cache type: {}, final cache size: {}", configKey, cache.size());
  } catch (Exception e) {
      log.error("Failed to deserialize cache data for {}", configKey, e);
  }
}

I'd like to move towards a design structure where Rs2CacheManager and CacheSerializationManager are less tightly coupled with all of the classes extending Rs2Cache. When a new Cache is designed it should automatically (or with the least effort possible) register to the Rs2CacheManager. I'll post a couple of questions that can help us work towards a better implementation of the caching system.

• How can se of the polymorphic design that already exists in Rs2Cache?
• Since Guice is already used widely throughout the repo, shouldn't we also make use of it here for better dependency injection?
• Is it overkill to use reflection to recognize all cache modules and automatically register them?

[Voxsylvae]

Thanks for opening this discussion and taking the time to write up such a analysis of the caching system. You've identified some architectural problems that I've working on.

Looking at the current tight coupling issues, I'm already in the process of refactoring the caching system in the following direction:

Proposed Changes (In Progress)

Problem Analysis

The current Rs2CacheManager and CacheSerializationManager violate the Open-Closed Principle by requiring manual updates for every new cache type. This creates maintenance overhead and scaling issues.

Solution Direction: Registry Pattern with Self-Describing Caches

I've already started implementing these changes and am working through the following approach:

1. Rs2Cache with Self-Description

I'm extending the existing Rs2Cache<K,V> base class to make caches declare their own capabilities:

public abstract class Rs2Cache<K, V> {
    // Existing code...
    
    public abstract CacheSerializer<K, V> getSerializer();
    public abstract Set<CacheCapability> getCapabilities();
    public void performAsyncUpdate() { /* optional override */ }
}

public enum CacheCapability {
    PERSISTENCE_ENABLED,
    ASYNC_UPDATE_REQUIRED,
    SCENE_DEPENDENT,
    PROFILE_SPECIFIC
}

2. Implement Explicit Cache Registry

Rather than automatic discovery, I'm implementing explicit registration for predictable behavior:

@Singleton
public class CacheRegistry {
    private final Map<String, Rs2Cache<?, ?>> caches = new LinkedHashMap<>();
    
    @Inject
    public CacheRegistry() {
        // Explicit registration - maintains full control
        register("skills", Rs2SkillCache.getInstance());
        register("quests", Rs2QuestCache.getInstance());
        register("varbits", Rs2VarbitCache.getInstance());
        // Add new caches here - single point of registration
    }
    
    public Set<Rs2Cache<?, ?>> getCachesWithCapability(CacheCapability capability) {
        return caches.values().stream()
                .filter(cache -> cache.getCapabilities().contains(capability))
                .collect(Collectors.toSet());
    }
}

3. Refactor Managers to Use Capabilities

I'm eliminating hardcoded cache lists and switch statements:

// Current problematic approach:
private static void loadPersistentCaches(String profileKey) {
    // Manual enumeration of each cache type...
    if (Rs2SkillCache.getCache().isPersistenceEnabled()) {
        CacheSerializationManager.loadCache(Rs2SkillCache.getCache()...);
    }
    // Repeat for each cache type
}

// New capability-driven approach I'm implementing:
private static void loadPersistentCaches(String profileKey) {
    Set<Rs2Cache<?, ?>> persistentCaches = 
        registry.getCachesWithCapability(PERSISTENCE_ENABLED);
        
    for (Rs2Cache<?, ?> cache : persistentCaches) {
        CacheSerializationManager.loadCache(cache, profileKey);
        
        if (cache.getCapabilities().contains(ASYNC_UPDATE_REQUIRED)) {
            CompletableFuture.runAsync(() -> cache.performAsyncUpdate());
        }
    }
}

4. Generic Serialization Management

I'm replacing the switch-statement-based serialization with polymorphic serializers:

// Remove hardcoded switch statements in CacheSerializationManager
public static <K, V> void saveCache(Rs2Cache<K, V> cache, String profileKey) {
    if (!cache.isPersistenceEnabled()) return;
    
    CacheSerializer<K, V> serializer = cache.getSerializer();
    String json = serializer.serialize(cache);
    // Use cache.getConfigKey() instead of hardcoded strings
}

Why This Approach

Avoiding Full Automation

I'm deliberately not using reflection-based auto-discovery because:

• Debugging complexity: Automatic registration makes cache lifecycle harder to trace
• Environmental control: Different deployments may need different cache sets
• Build-time safety: Explicit registration catches missing dependencies at compile time
• Performance: No runtime classpath scanning overhead

Benefits of Explicit Registration

• Single point of control: Add new caches in one place (registry constructor)
• Compile-time validation: Missing serializers cause build failures
• Clear dependencies: Easy to see what's active in any environment
• Test flexibility: Simple to create test configurations with different cache sets

Current Progress

I've already begun implementing:

• Enhanced Rs2Cache base architecture with strategy patterns
• Persistence framework with CacheSerializationManager
• Working on capability system design
• Working on registry pattern implementation
• Still refactoring managers to remove switch statements

My Expected Outcome

• New cache integration: Declare capabilities + single registry line
• Manager simplification: No more hardcoded cache knowledge
• Improved testability: Easy to mock registry with specific cache sets
• Better separation of concerns: Caches own their serialization logic

This approach maintains explicit control while eliminating the tight coupling and scaling issues in the current implementation. The refactoring is designed to leverage the existing solid foundation while addressing the architectural concerns you made and i also have already seen.

[Krulvis]

Nice, that sounds like a very valuable improvement!

[Krulvis]

I'm glad we have this Discussions page and I am not again creating PRs on something that you have started working on already ;)