Console modify hp mana etc fix by bozimmerman · Pull Request #250 · mangoszero/server

bozimmerman · 2026-03-04T21:17:05Z

The Modify hp/mana/etc admin console commands were just broken. They were treating the argument as both current and maximum, and the net result is that you were always changing only the maximum value.

This fixes it by 1. correctly extracting the proper values separately, and 2. treating the max as optional, since you are usually just wanting to change the current value anyway.

This change is

Copilot

Pull request overview

Fixes broken admin console “modify” commands so they correctly treat the first argument as the current value and the (optional) second argument as the maximum value, instead of incorrectly using the first arg for both.

Changes:

Update .modify hp to parse current HP and optional max HP separately.
Update .modify mana to parse current mana and optional max mana separately.
Update .modify energy / .modify rage to parse scaled values correctly and make max optional.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-04T21:20:48Z

src/game/ChatCommands/PlayerCommands.cpp

+    int32 energym = (!ExtractInt32(&args, energyRawM) || energyRawM * 10 < energy)
+        ? (int32)chr->GetMaxPower(POWER_ENERGY)
+        : energyRawM * 10;


energyRawM * 10 is evaluated in the condition and again in the ternary result; it can overflow signed int32 (UB) and then be passed to SetMaxPower as a huge uint32. Compute once using a wider type and range-check (<= INT32_MAX/10) before comparing/assigning.

@copilot open a new pull request to apply changes based on this feedback

Copilot · 2026-03-04T21:20:48Z

src/game/ChatCommands/PlayerCommands.cpp

        SetSentErrorMessage(true);
        return false;
    }
+    int32 rage = rageRaw * 10;


rageRaw * 10 can overflow signed int32 (undefined behavior) and then flow into SetPower/SetMaxPower via implicit conversion to uint32. Add a range check (rageRaw <= INT32_MAX/10) and multiply using a wider type before casting.

@copilot open a new pull request to apply changes based on this feedback

Copilot · 2026-03-04T21:20:48Z

src/game/ChatCommands/PlayerCommands.cpp

+    int32 ragem = (!ExtractInt32(&args, rageRawM) || rageRawM * 10 < rage)
+        ? (int32)chr->GetMaxPower(POWER_RAGE)
+        : rageRawM * 10;


rageRawM * 10 is evaluated multiple times and can overflow signed int32 (UB), which can then set an enormous uint32 max power. Compute the scaled value once using a wider type and validate it is within range before comparing/assigning.

src/game/ChatCommands/PlayerCommands.cpp

Copilot · 2026-03-04T21:20:49Z

src/game/ChatCommands/PlayerCommands.cpp

+    if (!ExtractInt32(&args, manam) || manam < mana)
+        manam = (int32)chr->GetMaxPower(POWER_MANA);


manam has the same issue as HP: a non-numeric second arg or a max smaller than mana is treated as “max omitted” and silently replaced with the current max mana. If the user provided a second arg and it fails to parse or is < mana, this should report LANG_BAD_VALUE (use ExtractOptInt32 only for the truly-omitted case).

Suggested change

if (!ExtractInt32(&args, manam) || manam < mana)

manam = (int32)chr->GetMaxPower(POWER_MANA);

// If no second argument is provided, use current max mana.

if (!*args)

{

manam = (int32)chr->GetMaxPower(POWER_MANA);

}

else

{

// Second argument was provided: it must be a valid integer >= mana.

if (!ExtractInt32(&args, manam) || manam < mana)

{

SendSysMessage(LANG_BAD_VALUE);

SetSentErrorMessage(true);

return false;

}

}

Copilot · 2026-03-04T21:20:49Z

src/game/ChatCommands/PlayerCommands.cpp

        SetSentErrorMessage(true);
        return false;
    }
+    int32 energy = energyRaw * 10;


energyRaw * 10 can overflow signed int32. Signed overflow is undefined behavior in C++, and a wrapped negative value could later be converted to a huge uint32. Add an explicit range check (e.g., energyRaw > INT32_MAX/10) and compute in a wider type before casting.

AppVeyorBot · 2026-03-04T22:18:42Z

✅ Build Mangos ZERO Server 22.04.148-vvegckuc completed (commit ab7fab42c1 by @bozimmerman)

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

billy1arm · 2026-03-05T00:02:40Z

@copilot open a new pull request to apply changes based on the comments in this thread

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 9 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Copilot · 2026-03-05T00:08:14Z

src/game/ChatCommands/PlayerCommands.cpp

+    int32 hp;
+    if (!ExtractInt32(&args, hp) || hp <= 0)


If the max value is omitted and hp is greater than the character’s existing max health, hpm remains the old max and the hpm < hp validation never runs. This can lead to setting current HP above max (or relying on downstream clamping). Consider validating hpm >= hp regardless of whether the optional arg was provided, or (if intended) auto-bumping hpm to at least hp when max is omitted.

Copilot · 2026-03-05T00:08:14Z

src/game/ChatCommands/PlayerCommands.cpp

+    if (ExtractOptInt32(&args, hpm))
+    {
+        if (hpm < hp)
+        {
+            SendSysMessage(LANG_BAD_VALUE);
+            SetSentErrorMessage(true);
+            return false;
+        }
+    }
+
    PSendSysMessage(LANG_YOU_CHANGE_HP, GetNameLink(chr).c_str(), hp, hpm);


If the max value is omitted and hp is greater than the character’s existing max health, hpm remains the old max and the hpm < hp validation never runs. This can lead to setting current HP above max (or relying on downstream clamping). Consider validating hpm >= hp regardless of whether the optional arg was provided, or (if intended) auto-bumping hpm to at least hp when max is omitted.

Suggested change

if (ExtractOptInt32(&args, hpm))

{

if (hpm < hp)

{

SendSysMessage(LANG_BAD_VALUE);

SetSentErrorMessage(true);

return false;

}

}

PSendSysMessage(LANG_YOU_CHANGE_HP, GetNameLink(chr).c_str(), hp, hpm);

ExtractOptInt32(&args, hpm);

if (hpm < hp)

{

SendSysMessage(LANG_BAD_VALUE);

SetSentErrorMessage(true);

return false;

}

PSendSysMessage(LANG_YOU_CHANGE_HP, GetNameLink(chr).c_str(), hp, hpm);

Copilot · 2026-03-05T00:08:15Z

src/game/ChatCommands/PlayerCommands.cpp

+    int32 manam;
+    if (!ExtractInt32(&args, manam) || manam < mana)
+        manam = (int32)chr->GetMaxPower(POWER_MANA);


This makes the max-mana argument effectively required (it calls ExtractInt32, not an optional extractor), and it also silently falls back to the existing max when the provided manam is smaller than mana. That can hide bad inputs and is inconsistent with the HP path (which errors when a provided max is too small). Consider using an optional extraction pattern here and rejecting (with LANG_BAD_VALUE) explicitly provided manam < mana.

Copilot · 2026-03-05T00:08:15Z

src/game/ChatCommands/PlayerCommands.cpp

+    int32 energym = (!ExtractInt32(&args, energyRawM) || energyRawM * 10 < energy)
+        ? (int32)chr->GetMaxPower(POWER_ENERGY)
+        : energyRawM * 10;
+


For energy/rage, providing a max value smaller than the current value silently falls back to the existing max instead of reporting a bad value. This changes the previous behavior (which rejected max < current) and can make operator mistakes hard to detect. Consider treating max as optional (omitted => default), but if it is present and < current, return LANG_BAD_VALUE (similar to the HP handling).

Suggested change

int32 energym = (!ExtractInt32(&args, energyRawM) || energyRawM * 10 < energy)

? (int32)chr->GetMaxPower(POWER_ENERGY)

: energyRawM * 10;

int32 energym;

// Max energy is optional: if omitted, keep current max; if provided and < current, report bad value.

if (!ExtractInt32(&args, energyRawM))

{

energym = (int32)chr->GetMaxPower(POWER_ENERGY);

}

else

{

energym = energyRawM * 10;

if (energym < energy)

{

SendSysMessage(LANG_BAD_VALUE);

SetSentErrorMessage(true);

return false;

}

}

Copilot · 2026-03-05T00:08:15Z

src/game/ChatCommands/PlayerCommands.cpp

+    int32 ragem = (!ExtractInt32(&args, rageRawM) || rageRawM * 10 < rage)
+        ? (int32)chr->GetMaxPower(POWER_RAGE)
+        : rageRawM * 10;
+


For energy/rage, providing a max value smaller than the current value silently falls back to the existing max instead of reporting a bad value. This changes the previous behavior (which rejected max < current) and can make operator mistakes hard to detect. Consider treating max as optional (omitted => default), but if it is present and < current, return LANG_BAD_VALUE (similar to the HP handling).

Suggested change

int32 ragem = (!ExtractInt32(&args, rageRawM) || rageRawM * 10 < rage)

? (int32)chr->GetMaxPower(POWER_RAGE)

: rageRawM * 10;

int32 ragem;

// Optional max rage argument: if provided, it must not be less than current rage.

if (!ExtractInt32(&args, rageRawM))

{

// No max provided, keep current max rage.

ragem = (int32)chr->GetMaxPower(POWER_RAGE);

}

else

{

ragem = rageRawM * 10;

if (ragem < rage)

{

SendSysMessage(LANG_BAD_VALUE);

SetSentErrorMessage(true);

return false;

}

}

Copilot · 2026-03-05T00:08:15Z

src/game/ChatCommands/PlayerCommands.cpp

        SetSentErrorMessage(true);
        return false;
    }
+    int32 energy = energyRaw * 10;


Multiplying user-provided *Raw values by 10 can overflow int32 (e.g., large inputs), which would break validation and could set unintended values. Consider bounding the allowed raw input (e.g., check raw <= INT32_MAX / 10 and similarly for negative if allowed) before multiplying.

Copilot · 2026-03-05T00:08:15Z

src/game/ChatCommands/PlayerCommands.cpp

    }

+    int32 energyRawM;
+    int32 energym = (!ExtractInt32(&args, energyRawM) || energyRawM * 10 < energy)


Multiplying user-provided *Raw values by 10 can overflow int32 (e.g., large inputs), which would break validation and could set unintended values. Consider bounding the allowed raw input (e.g., check raw <= INT32_MAX / 10 and similarly for negative if allowed) before multiplying.

Copilot · 2026-03-05T00:08:15Z

src/game/ChatCommands/PlayerCommands.cpp

        SetSentErrorMessage(true);
        return false;
    }
+    int32 rage = rageRaw * 10;


Multiplying user-provided *Raw values by 10 can overflow int32 (e.g., large inputs), which would break validation and could set unintended values. Consider bounding the allowed raw input (e.g., check raw <= INT32_MAX / 10 and similarly for negative if allowed) before multiplying.

Copilot · 2026-03-05T00:08:16Z

src/game/ChatCommands/PlayerCommands.cpp

    }

+    int32 rageRawM;
+    int32 ragem = (!ExtractInt32(&args, rageRawM) || rageRawM * 10 < rage)


Multiplying user-provided *Raw values by 10 can overflow int32 (e.g., large inputs), which would break validation and could set unintended values. Consider bounding the allowed raw input (e.g., check raw <= INT32_MAX / 10 and similarly for negative if allowed) before multiplying.

AppVeyorBot · 2026-03-05T00:14:17Z

❌ Build Mangos ZERO Server 22.04.150-geowwtky failed (commit a867847d1d by @billy1arm)

console modify hp mana etc fix

cd30a1a

Copilot AI review requested due to automatic review settings March 4, 2026 21:17

Copilot started reviewing on behalf of bozimmerman March 4, 2026 21:17 View session

Copilot AI reviewed Mar 4, 2026

View reviewed changes

Update src/game/ChatCommands/PlayerCommands.cpp

7aad8dc

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings March 5, 2026 00:02

billy1arm merged commit ce303ee into mangoszero:master Mar 5, 2026
3 of 9 checks passed

Copilot AI reviewed Mar 5, 2026

View reviewed changes

Copilot started reviewing on behalf of billy1arm March 5, 2026 00:13 View session

bozimmerman deleted the console_modify_cmd_fixes branch March 8, 2026 03:21

		if (!ExtractInt32(&args, manam) \|\| manam < mana)
		manam = (int32)chr->GetMaxPower(POWER_MANA);

-    int32 ragem = (!ExtractInt32(&args, rageRawM) || rageRawM * 10 < rage)
-        ? (int32)chr->GetMaxPower(POWER_RAGE)
-        : rageRawM * 10;
+    int32 ragem;
+    // Optional max rage argument: if provided, it must not be less than current rage.
+    if (!ExtractInt32(&args, rageRawM))
+    {
+        // No max provided, keep current max rage.
+        ragem = (int32)chr->GetMaxPower(POWER_RAGE);
+    }
+    else
+    {
+        ragem = rageRawM * 10;
+        if (ragem < rage)
+        {
+            SendSysMessage(LANG_BAD_VALUE);
+            SetSentErrorMessage(true);
+            return false;
+        }
+    }

Conversation

bozimmerman commented Mar 4, 2026 • edited by billy1arm Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

billy1arm Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

billy1arm Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

AppVeyorBot commented Mar 4, 2026

Uh oh!

billy1arm commented Mar 5, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

AppVeyorBot commented Mar 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

bozimmerman commented Mar 4, 2026 •

edited by billy1arm

Loading