Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -97,19 +97,14 @@ public static SpiritTreeData getSpiritTreeData(SpiritTree spiritTree) {
return getInstance().get(spiritTree, () -> {
try {
// Determine initial state based on spiritTree type
CropState cropState = null;
boolean availableForTravel = false;
CropState cropState = CropState.HARVESTABLE;
boolean availableForTravel = spiritTree.hasQuestRequirements();

if (spiritTree.getType() == SpiritTree.SpiritTreeType.BUILT_IN) {
// Built-in trees are available if quest requirements are met
availableForTravel = spiritTree.hasQuestRequirements();
} else if (spiritTree.getType() == SpiritTree.SpiritTreeType.FARMABLE) {
// Farmable trees - get current farming state
if (spiritTree.getType() == SpiritTree.SpiritTreeType.FARMABLE) {
cropState = spiritTree.getPatchState();
availableForTravel = spiritTree.isAvailableForTravel();
}else if (spiritTree.getType() == SpiritTree.SpiritTreeType.POH) {
// Other types (e.g. POH spirit trees) - use default availability
availableForTravel = spiritTree.isAvailableForTravel();
availableForTravel &= spiritTree.isAvailableForTravel();
} else if (spiritTree.getType() == SpiritTree.SpiritTreeType.POH) {
availableForTravel &= spiritTree.hasLevelRequirement();
}

log.debug("Initial spirit tree data for {}: \n\tcropState={}, available={}",
Expand Down Expand Up @@ -590,6 +585,14 @@ public void onGameObjectSpawned(net.runelite.api.events.GameObjectSpawned event)
getInstance().handleEvent(event);
}

/**
* Handle GameObjectSpawned event and delegate to update strategy.
*/
@Subscribe
public void onGameObjectDespawned(net.runelite.api.events.GameObjectDespawned event) {
getInstance().handleEvent(event);
}

/**
* Handle game state changes for cache lifecycle management (unchanged).
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,13 @@
import net.runelite.api.GameObject;
import net.runelite.api.Skill;
import net.runelite.api.coords.WorldPoint;
import net.runelite.api.events.GameStateChanged;
import net.runelite.api.events.VarbitChanged;
import net.runelite.api.events.WidgetLoaded;
import net.runelite.api.events.*;
import net.runelite.api.gameval.ObjectID;
import net.runelite.api.gameval.VarbitID;
import net.runelite.api.widgets.Widget;
import net.runelite.client.plugins.microbot.Microbot;
import net.runelite.client.plugins.microbot.questhelper.helpers.mischelpers.farmruns.CropState;
import net.runelite.client.plugins.microbot.util.cache.Rs2Cache;
import net.runelite.client.plugins.microbot.util.cache.Rs2ObjectCache;
import net.runelite.client.plugins.microbot.util.cache.model.SpiritTreeData;
import net.runelite.client.plugins.microbot.util.cache.strategy.CacheOperations;
import net.runelite.client.plugins.microbot.util.cache.strategy.CacheUpdateStrategy;
Expand Down Expand Up @@ -59,27 +56,41 @@ public class SpiritTreeUpdateStrategy implements CacheUpdateStrategy<SpiritTree,
VarbitID.FARMING_TRANSMIT_B, // 4772 - Etceteria and Brimhaven patches
VarbitID.FARMING_TRANSMIT_F // 7904 - Hosidius
);


/**
* Handle an event from the client and update the cache accordingly.
* @param event The event that occurred
* @param cache The cache to potentially update
*/
@Override
public void handleEvent(Object event, CacheOperations<SpiritTree, SpiritTreeData> cache) {
try {

if (event instanceof WidgetLoaded) {
handleWidgetLoaded((WidgetLoaded) event, cache);
} else if (event instanceof VarbitChanged) {
handleVarbitChanged((VarbitChanged) event, cache);
} else if (event instanceof GameStateChanged) {
handleGameStateChanged((GameStateChanged) event, cache);
} else if (event instanceof GameObjectSpawned) {
GameObject go = ((GameObjectSpawned) event).getGameObject();
handleGameObjectChange(go, true, cache);
} else if (event instanceof GameObjectDespawned) {
GameObject go = ((GameObjectDespawned) event).getGameObject();
handleGameObjectChange(go, false, cache);
}
} catch (Exception e) {
log.error("Error handling event in SpiritTreeUpdateStrategy: {}", e.getMessage(), e);
}
}


/**
* Get the event types that are handled by this strategy
*/
@Override
public Class<?>[] getHandledEventTypes() {
return new Class<?>[]{WidgetLoaded.class, VarbitChanged.class, GameStateChanged.class};
}
return new Class<?>[]{WidgetLoaded.class, VarbitChanged.class, GameStateChanged.class, GameObjectSpawned.class, GameObjectDespawned.class};
}

/**
* Handle widget loaded events to detect spirit tree widget opening
*/
Expand Down Expand Up @@ -127,13 +138,38 @@ private void handleVarbitChanged(VarbitChanged event, CacheOperations<SpiritTree
if(varbitId == VarbitID.POH_SPIRIT_TREE_UPROOTED){
log.debug("TODO update cache for POH spirit tree uprooted varbit change,currently in POH? {} ",PohTeleports.isInHouse());
}
if (Rs2Cache.isInPOH()) {
log.debug("Player in POH, checking for spirit tree objects,changed varbit {}", varbitId);
updatePOHSpiritTreeCache(cache);
}

/**
* Handles changes to game objects related to spirit trees and updates the cache accordingly.
* If the provided game object matches known spirit tree objects, it logs the detection
* and updates the cache with the relevant state.
*
* @param gameObject The game object that has changed. May be null, in which case this method does nothing.
* @param spawned Indicates whether the game object was spawned (true) or despawned (false).
* @param cache The cache instance used for storing and updating {@link SpiritTreeData} for detected spirit trees.
*/
private void handleGameObjectChange(GameObject gameObject, boolean spawned, CacheOperations<SpiritTree, SpiritTreeData> cache) {
if(gameObject == null){
return;
}

Arrays.stream(SpiritTree.values()).filter(tree -> tree.getType() == SpiritTree.SpiritTreeType.POH).forEach(tree -> {
if (tree.getObjectId().contains(gameObject.getId())) {
log.info("Found spirit tree object {} for POH spirit tree {}, {} to cache", gameObject.getId(), tree.name(), spawned ? "added" : "removed");
SpiritTreeData newData = new SpiritTreeData(
tree,
spawned ? CropState.HARVESTABLE: CropState.DEAD,
spawned,
gameObject.getWorldLocation(),
false, // Not detected via widget
true // Detected via nearby tree if present
);
cache.put(tree, newData);
}
});

}

/**
* Handle GameStateChanged events to detect POH region changes and validate spirit tree presence
*/
Expand All @@ -148,53 +184,11 @@ private void handleGameStateChanged(GameStateChanged event, CacheOperations<Spir
try {
// Use unified region detection from Rs2Cache
Rs2Cache.checkAndHandleRegionChange(cache);

// Check if we're in POH and validate spirit tree presence
if (Rs2Cache.isInPOH()) {
log.debug("Player in POH, checking for spirit tree objects");
updatePOHSpiritTreeCache(cache);
}
} catch (Exception e) {
log.error("Error handling GameStateChanged in SpiritTreeUpdateStrategy: {}", e.getMessage(), e);
}
}

/**
Copy link
Contributor

Choose a reason for hiding this comment

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

Why remove these functions entirely, and why can't we update the state based on the varbit for the spirit tree?We must wait for the object cache check to complete the scan and have populated the update. In the handleGameStateChanged, you cannot use the cache because it's not updated, so we would need to wait for the object cache to be updated on the object spawn and despawn events, also on a gamestate change event.

Copy link
Contributor Author

@Krulvis Krulvis Sep 9, 2025

Choose a reason for hiding this comment

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

You can use the varbit still. I didn't touch the part for POH spirit tree varbits that you left with //TODO to still be implemented:

if(varbitId == VarbitID.POH_SPIRIT_TREE_UPROOTED){
    log.debug("TODO update cache for POH spirit tree uprooted varbit change,currently in POH? {} ",PohTeleports.isInHouse());
}

What I removed from handleVarbitChange was just a proxy for scanning for gameobjects:

if (Rs2Cache.isInPOH()) {
       log.debug("Player in POH, checking for spirit tree objects,changed varbit {}", varbitId);
       updatePOHSpiritTreeCache(cache);
}

In the handleGameStateChanged, you cannot use the cache because it's not updated, so we would need to wait for the object cache to be updated on the object spawn and despawn events, also on a gamestate change event.

This is what I gave as a reason to remove scanning for objects in POH from the handleGameStateChanged function. I'm not sure what you're saying here exaclty.

Copy link
Contributor

Choose a reason for hiding this comment

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

You have to delay the scan, of course, until the data are stable. This is what I'm referring to.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You have to delay the scan, of course, until the data are stable. This is what I'm referring to.

Yes that would be an option. How long are you going to delay it for? Until the game object spawns preferably, right? So how about we just look at the gameobjectspawns event instead and skip the delaying setup altogether?

* Update cache for POH spirit tree based on object detection
*/
private void updatePOHSpiritTreeCache(CacheOperations<SpiritTree, SpiritTreeData> cache) {
try {
WorldPoint playerLocation = getPlayerLocation();
if (playerLocation == null|| !Rs2Cache.isInPOH()) {
log.warn("Cannot determine player location for POH spirit tree detection");
return;
}
// Stream over all SpiritTree values, filter for POH type, and update cache for each
Arrays.stream(SpiritTree.values())
.filter(tree -> tree.getType() == SpiritTree.SpiritTreeType.POH)
.forEach(tree -> {
// Check for POH spirit tree object nearby using the objectId defined in the enum
boolean pohSpiritTreePresent = Rs2ObjectCache.getGameObjects()
.anyMatch(obj -> tree.getObjectId().contains(obj.getId()) &&
obj.getWorldLocation().distanceTo(playerLocation) <= 40);

SpiritTreeData newData = new SpiritTreeData(
tree,
pohSpiritTreePresent ? CropState.HARVESTABLE : CropState.DEAD,
pohSpiritTreePresent,
playerLocation,
false, // Not detected via widget
pohSpiritTreePresent // Detected via nearby tree if present
);

cache.put(tree, newData);
log.debug("Updated POH spirit tree cache for {} - {}", tree.name(), pohSpiritTreePresent ? "tree present and available" : "no tree present");
});
} catch (Exception e) {
log.error("Error updating POH spirit tree cache: {}", e.getMessage(), e);
}
}

/**
* Update cache from spirit tree widget data
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import net.runelite.client.plugins.microbot.questhelper.helpers.mischelpers.farmruns.CropState;
import net.runelite.client.plugins.microbot.questhelper.helpers.mischelpers.farmruns.FarmingPatch;
import net.runelite.client.plugins.microbot.util.cache.Rs2SpiritTreeCache;
import net.runelite.client.plugins.microbot.util.gameobject.Rs2GameObject;
import net.runelite.client.plugins.microbot.util.misc.Rs2UiHelper;
import net.runelite.client.plugins.microbot.util.player.Rs2Player;
import net.runelite.client.plugins.microbot.util.poh.PohTeleports;
Expand Down Expand Up @@ -199,9 +200,9 @@ public enum SpiritTree {
null, // Location is dynamic based on player's house
SpiritTreeType.POH,
List.of(Quest.TREE_GNOME_VILLAGE, Quest.THE_GRAND_TREE, Quest.FAIRYTALE_II__CURE_A_QUEEN),
75, // Requires 75 Construction
95, // Requires 95 Construction
VarbitID.POH_SPIRIT_TREE_UPROOTED, // TODO Must be checkedPOH Spirit Tree varbit (combined tree/ring uses same varbit)
List.of(ObjectID.POH_SPIRIT_RING), // TODO Must be checked Spirit Ring object ID // we must update it. here are also variations for it.. leauge skins
Copy link
Contributor

Choose a reason for hiding this comment

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

Why use Rs2GameObject here and not the actual IDs from ObjectID? Not using the variations and also only the spirit tree variation with the fairy ring.
only using "poh_spirit_ring" is the wrong approach here. - is it not only the spirit tree with the fairy ring? What about the spirit tree without the fairy ring? Here we must consider not only https://oldschool.runescape.wiki/w/Spirit_tree_%26_fairy_ring, but we should add all variations for https://oldschool.runescape.wiki/w/Spirit_tree_(Construction) and the Spirit tree & fairy ring.https://oldschool.runescape.wiki/w/Spirit_tree_(Construction) -> why is the construction level changed to 95?
https://oldschool.runescape.wiki/w/Superior_garden

Copy link
Contributor Author

@Krulvis Krulvis Sep 9, 2025

Choose a reason for hiding this comment

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

Why use Rs2GameObject here and not the actual IDs from ObjectID? Not using the variations and also only the spirit tree variation with the fairy ring.

Well, the Rs2GameObjects.getObjectIdsByName function returns a list of all IDS we're interested in, which is exactly what you also want I believe?

public static List<Integer> getObjectIdsByName(String name) {

Copy link
Contributor Author

@Krulvis Krulvis Sep 9, 2025

Choose a reason for hiding this comment

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

The requirement for Spirit tree with fairy rings is 95 construction: https://oldschool.runescape.wiki/w/Spirit_tree_%26_fairy_ring

Copy link
Contributor

Choose a reason for hiding this comment

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

i see why it better to use the reflection approach here, because of the different IDs per configuration, great job on the changes otherwise.
Regarding the level. Sorry, my bad, have not seen the other one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i see why it better to use the reflection approach here, because of the different IDs per configuration, great job on the changes otherwise. Regarding the level. Sorry, my bad, have not seen the other one.

Aah I understand your confusion. It is unclear from the diff that I am changing only the Spirit Rings, not Spirit Tree enum value.

Rs2GameObject.getObjectIdsByName("poh_spirit_ring"),
new int[] {-1}, // Region must be player's own house
"Your house"
);
Expand Down Expand Up @@ -249,16 +250,6 @@ public boolean isAvailableForTravel() {
return isPatchHealthyAndGrown();
}

// Check construction level for POH trees
Copy link
Owner

Choose a reason for hiding this comment

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

Can you elaborate why you removed this? I'm not fully understanding this change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed it from the function isAvailableForTravel(). Your construction or farming levels have nothing to do with whether you can use a tree for travel. It either exists or it doesn't. And when it does, you can use it.
There are many people that boost their farming/construction levels to plant the tree. As such, this lead to unintended blocking behavior.

Additional rant:
I would even go as far as saying that all the level requirements in this file should be removed as planting spirit trees comes with higher level requirements the more trees you plant. "At level 83 Farming, players can only have one spirit tree planted at a time, not including the one in their player-owned house. Level 91 Farming allows for two spirit trees to be planted at a time, whilst 99 Farming allows for unlimited spirit trees." But -to make things more complex- the tree in your PoH does not count towards this. "Players can plant their own spirit tree from a spirit sapling in their Superior Garden at level 75 Construction. Doing so also requires level 83 in Farming. Boosts can be used. This tree does not count towards the number of spirit trees that may be planted at one time."

There is also a hasLevelRequirement() function where the level is being looked at, but that doesn't consider the amount of trees you have and uses Rs2Player.getRealSkillLevel(Skill.FARMING) >= requiredSkillLevel instead of getBoostedSkillLevel(). None of this code is used anywhere. If it would be used for actual farming, it wouldn't work.

Copy link
Owner

Choose a reason for hiding this comment

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

I removed it from the function isAvailableForTravel(). Your construction or farming levels have nothing to do with whether you can use a tree for travel. It either exists or it doesn't. And when it does, you can use it. There are many people that boost their farming/construction levels to plant the tree. As such, this lead to unintended blocking behavior.

Additional rant: I would even go as far as saying that all the level requirements in this file should be removed as planting spirit trees comes with higher level requirements the more trees you plant. "At level 83 Farming, players can only have one spirit tree planted at a time, not including the one in their player-owned house. Level 91 Farming allows for two spirit trees to be planted at a time, whilst 99 Farming allows for unlimited spirit trees." But -to make things more complex- the tree in your PoH does not count towards this. "Players can plant their own spirit tree from a spirit sapling in their Superior Garden at level 75 Construction. Doing so also requires level 83 in Farming. Boosts can be used. This tree does not count towards the number of spirit trees that may be planted at one time."

There is also a hasLevelRequirement() function where the level is being looked at, but that doesn't consider the amount of trees you have and uses Rs2Player.getRealSkillLevel(Skill.FARMING) >= requiredSkillLevel instead of getBoostedSkillLevel(). None of this code is used anywhere. If it would be used for actual farming, it wouldn't work.

Makes sense

if (type == SpiritTreeType.POH) {
if (Rs2Player.getRealSkillLevel(Skill.CONSTRUCTION) < requiredSkillLevel || !Rs2Farming.hasRequiredFarmingLevel(75)) {
return false;
}

// For POH trees, check if they are built and player is in their house
return isPOHTreeAvailable();
}

// Built-in trees are always available if quest requirements are met
return true;
}
Expand Down