Repository navigation
fix: hidden-GM targets (Soulstone, heals, group buffs), EnableBroadcasts=0 gaps, bag-swap crash, bug form (#1107, #1108) - #1109
Merged
Conversation
A warlock bot started the Soulstone every few seconds and never finished it when the player was a GM with visibility off: the core drops an invisible GM as a spell target (Spell::CheckTarget), so the cast never landed, the member never got the buff and the value offered them again. Reproduced live; with a normal visible player the bot casts the stone once. The warlock now skips invisible members when it picks a soulstone target. Its own buff check also looks for the Soulstone Resurrection aura, as the member check does; the old SPELL_EFFECT_SELF_RESURRECT test matched the "Use Soulstone" spell, which is never an aura.
AiPlayerbot.EnableBroadcasts = 0 only pushed broadcastChanceMaxValue to 0, so
the chance-gated broadcasts stopped while everything the bots say on their
own kept going: /say and /yell chatter, guild and world channel messages,
recruitment and invite lines, the flavour text of an emote, the scripted
battleground callouts and the rpg dialogue lines.
One central gate decides now: PlayerbotAI::Say, Yell, SayToGuild and the
named channel helpers refuse to send when broadcasts are off. Every
broadcast (BroadcastHelper), the "say" strategy quotes and the public chat
replies run through them, so they are silenced in one place. The chat that
sends straight to the core (the guild invite answers, the petition offer,
the recruitment lines, the group invite lines, the "thanks for the invite",
the trade thanks, the reveal line, the battleground callout, the emote
flavour text and the rpg dialogue) is gated at the line itself, so the
functional part of each action stays untouched.
What stays: whispers, party and raid chat, the answer to a player who
ordered the bot (".bot speak", a reply to the bot's own master) and the
TortoiseBotsManager addon protocol. Those are functional, not broadcasts.
Builds on the fix proposed by Ildourol in
Ildourol@76a0879
reviewed hunk by hunk. Dropped: the broadcastChanceMaxValue change to 1 (it
makes every chance roll succeed instead of fail, which would turn the
broadcasts on), the RevealGatheringItemAction early return (it would also
drop the ping the group needs), the redundant per-caller gates that the
central gate already covers, and the ChatReplyAction gate in the form that
silences the reply to the bot's own master.
…ion list EquipAction moved the contents of a replaced bag with Bag::RemoveItem and then Player::StoreItem. StoreItem registers a timed item (quest items with a duration) in the player's duration list again, while the bag-only removal never took it out, so the item sat in the list twice. When it expired, DestroyItem dropped one entry and freed the item; the second entry dangled and the next Player::Update crashed in Item::UpdateDuration (SIGSEGV). Player::RemoveItem keeps the list in step, as the core's own moves do.
The shared party-member check already skips a player with visibility off, because the core drops a hidden GM as a spell target and the cast never lands. The heal target value, the snared-member value and the group-buff missing count ran their own checks without it, so a healer could keep casting at a hidden GM and a group buff stayed wanted forever.
#1107 turned out to be a hidden GM, which the report did not say. The bug form now asks whether the reporter played on a GM account and for the setup (characters, bot types, group, changed settings, how often).
Sagiroth
changed the title
Sagiroth
marked this pull request as ready for review
October 19, 2026 21:23
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three fixes: endless Soulstone, EnableBroadcasts, and a world-server crash.
Fixes #1107
Fixes #1108
Warlock casts Soulstone nonstop (#1107)
Cause. When the player was a GM with visibility off, the core silently drops them as a spell target. The Soulstone never landed, the player never got the buff, and the warlock started the cast again every ~5 seconds. I reproduced it live: as an invisible GM the cast restarted every 5 s; as a normal, visible player the warlock stoned me once and stopped.
Changed
docs/classes/warlock.mdmentions the invisible-GM rule.AiPlayerbot.EnableBroadcasts = 0did not silence all bot chat (#1108)This builds on @Ildourol's proposed commit (thank you!).
Changed
Still works with broadcasts off: whispers, party/raid chat, answers to commands,
speak, replies to the bot's own master, and TortoiseBotsManager addon messages.Differences from the proposed commit
broadcastChanceMaxValueat0. Setting it to1would make every chance-based broadcast fire, becauseurand(1, 1)is always 1.Tested live with broadcasts on and off:
speak helloand a whisperedstatscommand still work with broadcasts off.Crash: SIGSEGV in
Item::UpdateDurationCause. When a bot equips a bigger bag over a non-empty one,
EquipActionmoved the old bag's contents withBag::RemoveItemand thenPlayer::StoreItem.StoreItemadds a timed item (quest items with a duration) to the player's duration list again, but the bag-only removal never took it off, so the item was listed twice. When the item expired, the core destroyed it and removed one entry. The other entry pointed to freed memory, and the next player update crashed.Changed: the move uses
Player::RemoveItem, which keeps the list in step, the same way the core's own item moves do.Not reproduced on demand. The crash happened once in my test run, and the stack trace matches this path. The build is clean and the server ran normally afterwards.
Other targets skip a hidden GM too
The shared party-member check already skipped a hidden GM. Three places picked targets on their own without that check, and I added it to each: the heal target, the member to free from a snare or root, and the count of members missing a group buff. Without it a healer could keep casting at a hidden GM, and a group buff stayed wanted forever. The build is clean. I did not run these three in a live test.
Bug report form
The bug form now asks whether the reporter played on a GM account, and for their setup: characters, bot types, group, changed settings, and how often it happens. #1107 turned out to be a hidden GM, and the report did not say so.
Checks
tools/verify_all.shpasses.