FFXIV-CombatReborn / GatherBuddyReborn

Dalamud-based FFXIV addon to extremely simplify gathering.
Apache License 2.0
31 stars 29 forks source link

Adding experimental k-means clustering to AutoGather node selection #187

Open phecdaDia opened 2 weeks ago

phecdaDia commented 2 weeks ago

Adding a new optional node selection using k-means clustering to avoid going to the same "cluster" of nodes when they have not yet respawned. Restricted it's usage to nodes of level 51 or higher, since ARR nodes do not follow the pattern for 2 gatherable nodes in a cluster, as well as only normal nodes. Tested on multiple Dawntrail gatherables and tested without the option enabled to ensure there should be no regression. Current limit for iterations of the k-means algorithm is set to 100, however it supports early stopping and usually aborts after only a few iterations.

NostraThomas99 commented 2 weeks ago

This PR reintroduces an old bug where GBR will bonk its head on an unavailable node if it picks an unavailable node during far node selection. Current behavior is to interrupt movement and go to the closest available node as soon as available nodes are visible. Looks to be related to the changes at line 463 in AutoGather.cs. If you can sort out that issue I can merge the PR.

phecdaDia commented 2 weeks ago

Hmm. The changed section should be logically identically to what was previously there, except the player distance check has been moved outside of the .Any() and it's split into two if statements because I did some logging. I did some more testing on it quickly and even adding the original code it would still bonk it's head for me, which is why I hadn't really thought of that. It seems that if the movement task is already queued it then attempts to enqueue another movement task after that to the valid node which goes ignored if the pathing hadn't been stopped before. I am pushing another fix in a bit which adds a check to the close node selection. This will affect current behaviour even with the new setting disabled. I have left the logging commands in there, albeit commented out. Please remove them if they're not needed. This new behaviour should also allow the bot to pick up clusters it's flying over, even if it first selected a different one. I quickly tested this on Bismuth ore since the nodes are relatively far apart the new check seems to trigger relatively often and changing the destination to a valid node.

phecdaDia commented 1 week ago

I have added another commit, to due some issues which arose with further testing, specifically stormblood content. A proper long-term solution which would also allow the feature to be stable would most likely be to add a per-location toggle for kmeans clustering.

lazyranma commented 1 week ago

This PR reintroduces an old bug where GBR will bonk its head on an unavailable node if it picks an unavailable node during far node selection. Current behavior is to interrupt movement and go to the closest available node as soon as available nodes are visible. Looks to be related to the changes at line 463 in AutoGather.cs. If you can sort out that issue I can merge the PR.

That's because MoveToCloseNode() and Navigate() won't reset navigation unless within 3 units from the visible node. I've run into the same issue on the main branch.

I made a simpler solution to the same problem. You need to remember the last 4 visited nodes (2 for ephemeral) and pick up a node furthest from all of them. The only problem is that the character would fly to the literally furthest node because of the aforementioned problem, but I'll look into it. https://github.com/FFXIV-CombatReborn/GatherBuddyReborn/pull/198/commits/9d2a2b5f499d00cc16fd1f56d5d4fca6f01cc6da