Skip to content

Bugfix/fix small bugs#8559

Open
sebaleme wants to merge 3 commits intodiasurgical:masterfrom
sebaleme:bugfix/fix-small-bugs
Open

Bugfix/fix small bugs#8559
sebaleme wants to merge 3 commits intodiasurgical:masterfrom
sebaleme:bugfix/fix-small-bugs

Conversation

@sebaleme
Copy link
Copy Markdown

@sebaleme sebaleme commented May 9, 2026

2 small fixes. The first one is probably not affecting the binary, since the compiler optimizing non read data, and the second has a very restricted effect.

Comment thread Source/missiles.cpp
const Player &player = Players[missile._misource];
int dmg = GenerateRnd(10) + (player.getCharacterLevel() / 2) + 1;
dmg = ScaleSpellEffect(dmg, missile._mispllvl);
int dmg = ScaleSpellEffect(dmg, missile._mispllvl);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The first one is probably not affecting the binary, since the compiler optimizing non read data

That's not correct. dmg was being used before your change, as it gets passed into ScaleSpellEffect(). Your compiler should be giving you a warning here.

[ 55%] Building CXX object Source/CMakeFiles/libdevilutionx.dir/missiles.cpp.o
.../DevilutionX/Source/missiles.cpp: In function ‘bool devilution::{anonymous}::GuardianTryFireAt(devilution::Missile&, devilution::Point)’:
.../DevilutionX/Source/missiles.cpp:729:23: warning: unused variable ‘player’ [-Wunused-variable]
  729 |         const Player &player = Players[missile._misource];
      |                       ^~~~~~
.../DevilutionX/Source/missiles.cpp:730:35: warning: ‘dmg’ may be used uninitialized [-Wmaybe-uninitialized]
  730 |         int dmg = ScaleSpellEffect(dmg, missile._mispllvl);
      |                   ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~
.../DevilutionX/Source/missiles.cpp:730:13: note: ‘dmg’ was declared here
  730 |         int dmg = ScaleSpellEffect(dmg, missile._mispllvl);
      |             ^~~

@qndel
Copy link
Copy Markdown
Member

qndel commented May 9, 2026

I'd simply close this PR after reading title/description like this ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants