From 31543a7c999ad5cdacbab35a9bd6d81e900c183a Mon Sep 17 00:00:00 2001 From: joris <joris.dauphin@gmail.com> Date: Mon, 27 May 2013 14:02:36 +0200 Subject: [PATCH] Clean up: - Use std::min/max. - remove useless test: delete null ptr is safe. --- src/action/action_built.cpp | 5 +- src/action/action_resource.cpp | 4 +- src/ai/ai.cpp | 6 +-- src/missile/missile_deathcoil.cpp | 81 ++++++++++++++--------------- src/network/net_lowlevel.cpp | 8 +-- src/sound/sound_server.cpp | 8 +-- src/sound/wav.cpp | 8 +-- src/spell/spell_adjustvital.cpp | 16 ++---- src/spell/spell_areaadjustvital.cpp | 11 +--- src/stratagus/player.cpp | 4 +- src/stratagus/script.cpp | 4 +- src/ui/mainscr.cpp | 4 +- src/ui/ui.cpp | 4 +- src/ui/widgets.cpp | 14 ++--- src/unit/unit_find.cpp | 18 ++----- src/unit/upgrade.cpp | 23 ++------ src/video/font.cpp | 4 +- src/video/graphic.cpp | 22 +++----- 18 files changed, 81 insertions(+), 163 deletions(-) diff --git a/src/action/action_built.cpp b/src/action/action_built.cpp index 465daaa9d..c1bbb41cb 100644 --- a/src/action/action_built.cpp +++ b/src/action/action_built.cpp @@ -281,10 +281,7 @@ static void Finish(COrder_Built &order, CUnit &unit) // This should happen when building unit with several peons // Maybe also with only one. // FIXME : Should be better to fix it in action_{build,repair}.c ? - if (unit.Variable[BUILD_INDEX].Value > unit.Variable[BUILD_INDEX].Max) { - // assume value is wrong. - unit.Variable[BUILD_INDEX].Value = unit.Variable[BUILD_INDEX].Max; - } + unit.Variable[BUILD_INDEX].Value = std::min(unit.Variable[BUILD_INDEX].Max, unit.Variable[BUILD_INDEX].Value); } /* virtual */ void COrder_Built::FillSeenValues(CUnit &unit) const diff --git a/src/action/action_resource.cpp b/src/action/action_resource.cpp index b0b221c63..388c44f0e 100644 --- a/src/action/action_resource.cpp +++ b/src/action/action_resource.cpp @@ -706,9 +706,7 @@ int COrder_Resource::GatherResource(CUnit &unit) // Target is not dead, getting resources. if (is_visible) { // Don't load more that there is. - if (addload > source->ResourcesHeld) { - addload = source->ResourcesHeld; - } + addload = std::min(source->ResourcesHeld, addload); unit.ResourcesHeld += addload; source->ResourcesHeld -= addload; } diff --git a/src/ai/ai.cpp b/src/ai/ai.cpp index f137fa812..2b1901e9c 100644 --- a/src/ai/ai.cpp +++ b/src/ai/ai.cpp @@ -504,10 +504,8 @@ void InitAiModule() void CleanAi() { for (int p = 0; p < PlayerMax; ++p) { - if (Players[p].Ai) { - delete Players[p].Ai; - Players[p].Ai = NULL; - } + delete Players[p].Ai; + Players[p].Ai = NULL; } } diff --git a/src/missile/missile_deathcoil.cpp b/src/missile/missile_deathcoil.cpp index 6aa74b1f1..4a60112d5 100644 --- a/src/missile/missile_deathcoil.cpp +++ b/src/missile/missile_deathcoil.cpp @@ -50,54 +50,51 @@ void MissileDeathCoil::Action() { this->Wait = this->Type->Sleep; - if (PointToPointMissile(*this)) { - Assert(this->SourceUnit != NULL); - CUnit &source = *this->SourceUnit; + if (PointToPointMissile(*this) == false) { + return; + } + Assert(this->SourceUnit != NULL); + CUnit &source = *this->SourceUnit; - if (source.Destroyed) { + if (source.Destroyed) { + return; + } + // source unit still exists + // + // Target unit still exists and casted on a special target + // + if (this->TargetUnit && !this->TargetUnit->Destroyed + && this->TargetUnit->CurrentAction() == UnitActionDie) { + HitUnit(&source, *this->TargetUnit, this->Damage); + if (source.CurrentAction() != UnitActionDie) { + source.Variable[HP_INDEX].Value += this->Damage; + source.Variable[HP_INDEX].Value = std::min(source.Variable[HP_INDEX].Max, source.Variable[HP_INDEX].Value); + } + } else { + // + // No target unit -- try enemies in range 5x5 // Must be parametrable + // + std::vector<CUnit *> table; + const Vec2i destPos = Map.MapPixelPosToTilePos(this->destination); + const Vec2i range(2, 2); + Select(destPos - range, destPos + range, table, IsEnemyWith(*source.Player)); + + if (table.empty()) { return; } - // source unit still exists - // - // Target unit still exists and casted on a special target - // - if (this->TargetUnit && !this->TargetUnit->Destroyed - && this->TargetUnit->CurrentAction() == UnitActionDie) { - HitUnit(&source, *this->TargetUnit, this->Damage); - if (source.CurrentAction() != UnitActionDie) { - source.Variable[HP_INDEX].Value += this->Damage; - if (source.Variable[HP_INDEX].Value > source.Variable[HP_INDEX].Max) { - source.Variable[HP_INDEX].Value = source.Variable[HP_INDEX].Max; - } - } - } else { - // - // No target unit -- try enemies in range 5x5 // Must be parametrable - // - std::vector<CUnit *> table; - const Vec2i destPos = Map.MapPixelPosToTilePos(this->destination); - const Vec2i range(2, 2); - Select(destPos - range, destPos + range, table, IsEnemyWith(*source.Player)); + const size_t n = table.size(); // enemy count + const int damage = std::min<int>(1, this->Damage / n); - if (table.empty()) { - return; - } - const size_t n = table.size(); // enemy count - const int damage = std::min<int>(1, this->Damage / n); - - // disperse damage between them - for (size_t i = 0; i != n; ++i) { - HitUnit(&source, *table[i], damage); - } - if (source.CurrentAction() != UnitActionDie) { - source.Variable[HP_INDEX].Value += this->Damage; - if (source.Variable[HP_INDEX].Value > source.Variable[HP_INDEX].Max) { - source.Variable[HP_INDEX].Value = source.Variable[HP_INDEX].Max; - } - } + // disperse damage between them + for (size_t i = 0; i != n; ++i) { + HitUnit(&source, *table[i], damage); + } + if (source.CurrentAction() != UnitActionDie) { + source.Variable[HP_INDEX].Value += this->Damage; + source.Variable[HP_INDEX].Value = std::min(source.Variable[HP_INDEX].Max, source.Variable[HP_INDEX].Value); } - this->TTL = 0; } + this->TTL = 0; } //@} diff --git a/src/network/net_lowlevel.cpp b/src/network/net_lowlevel.cpp index 1b469f2e9..aa8905691 100644 --- a/src/network/net_lowlevel.cpp +++ b/src/network/net_lowlevel.cpp @@ -689,9 +689,7 @@ void SocketSet::AddSocket(Socket socket) { Sockets.push_back(socket); SocketReady.push_back(0); - if (socket > MaxSockFD) { - MaxSockFD = socket; - } + MaxSockFD = std::max(MaxSockFD, socket); } /** @@ -714,9 +712,7 @@ void SocketSet::DelSocket(Socket socket) if (socket == MaxSockFD) { MaxSockFD = 0; for (i = Sockets.begin(); i != Sockets.end(); ++i) { - if (*i > MaxSockFD) { - MaxSockFD = *i; - } + MaxSockFD = std::max(this->MaxSockFD, *i); } } } diff --git a/src/sound/sound_server.cpp b/src/sound/sound_server.cpp index 9b92ec28c..aabfb88bc 100644 --- a/src/sound/sound_server.cpp +++ b/src/sound/sound_server.cpp @@ -206,9 +206,7 @@ static int MixSampleToStereo32(CSample *sample, int index, unsigned char volume, Assert(!(index & 1)); - if (size >= (sample->Len - index) * div / 2) { - size = (sample->Len - index) * div / 2; - } + size = std::min((sample->Len - index) * div / 2, size); size = ConvertToStereo32((char *)(sample->Buffer + index), (char *)buf, sample->Frequency, sample->SampleSize / 8, sample->Channels, @@ -410,9 +408,7 @@ int SetChannelVolume(int channel, int volume) } else { SDL_LockAudio(); - if (volume > MaxVolume) { - volume = MaxVolume; - } + volume = std::min(MaxVolume, volume); Channels[channel].Volume = volume; SDL_UnlockAudio(); diff --git a/src/sound/wav.cpp b/src/sound/wav.cpp index 9255bf34c..81528fb66 100644 --- a/src/sound/wav.cpp +++ b/src/sound/wav.cpp @@ -189,9 +189,7 @@ int CSampleWavStream::Read(void *buf, int len) this->Len += comp; } - if (this->Len < len) { - len = this->Len; - } + len = std::min(this->Len, len); memcpy(buf, this->Buffer + this->Pos, len); this->Pos += len; @@ -209,9 +207,7 @@ CSampleWavStream::~CSampleWavStream() int CSampleWav::Read(void *buf, int len) { - if (len > this->Len) { - len = this->Len; - } + len = std::min(this->Len, len); memcpy(buf, this->Buffer + this->Pos, len); this->Pos += len; diff --git a/src/spell/spell_adjustvital.cpp b/src/spell/spell_adjustvital.cpp index 188bece33..a041ebbe0 100644 --- a/src/spell/spell_adjustvital.cpp +++ b/src/spell/spell_adjustvital.cpp @@ -114,23 +114,15 @@ HitUnit(&caster, *target, -(castcount * hp)); } else { target->Variable[HP_INDEX].Value += castcount * hp; - if (target->Variable[HP_INDEX].Value < 0) { - target->Variable[HP_INDEX].Value = 0; - } + target->Variable[HP_INDEX].Value = std::max(target->Variable[HP_INDEX].Value, 0); } } else { target->Variable[HP_INDEX].Value += castcount * hp; - if (target->Variable[HP_INDEX].Value > target->Variable[HP_INDEX].Max) { - target->Variable[HP_INDEX].Value = target->Variable[HP_INDEX].Max; - } + target->Variable[HP_INDEX].Value = std::min(target->Variable[HP_INDEX].Max, target->Variable[HP_INDEX].Value); } target->Variable[MANA_INDEX].Value += castcount * mana; - if (target->Variable[MANA_INDEX].Value < 0) { - target->Variable[MANA_INDEX].Value = 0; - } - if (target->Variable[MANA_INDEX].Value > target->Variable[MANA_INDEX].Max) { - target->Variable[MANA_INDEX].Value = target->Variable[MANA_INDEX].Max; - } + clamp(&target->Variable[MANA_INDEX].Value, 0, target->Variable[MANA_INDEX].Max); + if (spell.RepeatCast) { return 1; } diff --git a/src/spell/spell_areaadjustvital.cpp b/src/spell/spell_areaadjustvital.cpp index 69c4c42a7..4d4e6d195 100644 --- a/src/spell/spell_areaadjustvital.cpp +++ b/src/spell/spell_areaadjustvital.cpp @@ -84,17 +84,10 @@ HitUnit(&caster, *target, -hp); } else { target->Variable[HP_INDEX].Value += hp; - if (target->Variable[HP_INDEX].Value > target->Variable[HP_INDEX].Max) { - target->Variable[HP_INDEX].Value = target->Variable[HP_INDEX].Max; - } + target->Variable[HP_INDEX].Value = std::min(target->Variable[HP_INDEX].Max, target->Variable[HP_INDEX].Value); } target->Variable[MANA_INDEX].Value += mana; - if (target->Variable[MANA_INDEX].Value < 0) { - target->Variable[MANA_INDEX].Value = 0; - } - if (target->Variable[MANA_INDEX].Value > target->Variable[MANA_INDEX].Max) { - target->Variable[MANA_INDEX].Value = target->Variable[MANA_INDEX].Max; - } + clamp(&target->Variable[MANA_INDEX].Value, 0, target->Variable[MANA_INDEX].Max); } return 0; } diff --git a/src/stratagus/player.cpp b/src/stratagus/player.cpp index 23607f7cc..0c7a2e105 100644 --- a/src/stratagus/player.cpp +++ b/src/stratagus/player.cpp @@ -896,9 +896,7 @@ void CPlayer::ChangeResource(const int resource, const int value, const bool sto const int fromStore = std::min(this->StoredResources[resource], abs(value)); this->StoredResources[resource] -= fromStore; this->Resources[resource] -= abs(value) - fromStore; - if (this->Resources[resource] < 0) { - this->Resources[resource] = 0; - } + this->Resources[resource] = std::max(this->Resources[resource], 0); } else { if (store && this->MaxResources[resource] != -1) { this->StoredResources[resource] += std::min(value, this->MaxResources[resource] - this->StoredResources[resource]); diff --git a/src/stratagus/script.cpp b/src/stratagus/script.cpp index 02451d918..98ccdc99e 100644 --- a/src/stratagus/script.cpp +++ b/src/stratagus/script.cpp @@ -1066,9 +1066,7 @@ std::string EvalString(const StringDesc *s) } if (s->D.Line.MaxLen) { maxlen = EvalNumber(s->D.Line.MaxLen); - if (maxlen < 0) { - maxlen = 0; - } + maxlen = std::max(maxlen, 0); } else { maxlen = 0; } diff --git a/src/ui/mainscr.cpp b/src/ui/mainscr.cpp index dc2713200..f0f514c0e 100644 --- a/src/ui/mainscr.cpp +++ b/src/ui/mainscr.cpp @@ -1310,9 +1310,7 @@ void UpdateTimer() GameTimer.Cycles += GameCycle - GameTimer.LastUpdate; } else { GameTimer.Cycles -= GameCycle - GameTimer.LastUpdate; - if (GameTimer.Cycles < 0) { - GameTimer.Cycles = 0; - } + GameTimer.Cycles = std::max(GameTimer.Cycles, 0l); } GameTimer.LastUpdate = GameCycle; } diff --git a/src/ui/ui.cpp b/src/ui/ui.cpp index 3478e91f2..b3a774501 100644 --- a/src/ui/ui.cpp +++ b/src/ui/ui.cpp @@ -432,9 +432,7 @@ static void FinishViewportModeConfiguration(CViewport new_vps[], int num_vps) // Update the viewport pointers // UI.MouseViewport = GetViewport(CursorScreenPos); - if (UI.SelectedViewport > UI.Viewports + UI.NumViewports - 1) { - UI.SelectedViewport = UI.Viewports + UI.NumViewports - 1; - } + UI.SelectedViewport = std::min(UI.Viewports + UI.NumViewports - 1, UI.SelectedViewport); } /** diff --git a/src/ui/widgets.cpp b/src/ui/widgets.cpp index da914722c..a3fed31fe 100644 --- a/src/ui/widgets.cpp +++ b/src/ui/widgets.cpp @@ -142,10 +142,8 @@ void freeGuichan() Gui = NULL; } - if (Input) { - delete Input; - Input = NULL; - } + delete Input; + Input = NULL; } /** @@ -548,9 +546,7 @@ void ImageRadioButton::adjustSize() if (uncheckedNormalImage) { width = uncheckedNormalImage->getWidth(); width += width / 2; - if (uncheckedNormalImage->getHeight() > height) { - height = uncheckedNormalImage->getHeight(); - } + height = std::max(height, uncheckedNormalImage->getHeight()); } else { width = getFont()->getHeight(); width += width / 2; @@ -683,9 +679,7 @@ void ImageCheckBox::adjustSize() if (uncheckedNormalImage) { width = uncheckedNormalImage->getWidth(); width += width / 2; - if (uncheckedNormalImage->getHeight() > height) { - height = uncheckedNormalImage->getHeight(); - } + height = std::max(height, uncheckedNormalImage->getHeight()); } else { width = getFont()->getHeight(); width += width / 2; diff --git a/src/unit/unit_find.cpp b/src/unit/unit_find.cpp index 5ca6b6ae9..f9f53b6f9 100644 --- a/src/unit/unit_find.cpp +++ b/src/unit/unit_find.cpp @@ -833,10 +833,8 @@ public: cost = HEALTH_FACTOR * (2 * hp_damage_evaluate - dest->Variable[HP_INDEX].Value) / (dtype.TileWidth * dtype.TileWidth); - if (cost < 1) { - cost = 1; - } - cost = (-cost); + cost = std::max(cost, 1); + cost = -cost; } else { // Priority 0-255 cost += dtype.Priority * PRIORITY_FACTOR; @@ -860,14 +858,8 @@ public: int effective_hp = (dest->Variable[HP_INDEX].Value - 2 * hp_damage_evaluate); // Unit we won't kill are evaluated the same - if (effective_hp > 0) { - effective_hp = 0; - } - // Unit we are sure to kill are all evaluated the same (except PRIORITY) - if (effective_hp < -hp_damage_evaluate) { - effective_hp = -hp_damage_evaluate; - } + clamp(&effective_hp, 0, -hp_damage_evaluate); // Here, effective_hp vary from -hp_damage_evaluate (unit will be killed) to 0 (unit can't be killed) // => we prefer killing rather than only hitting... @@ -880,9 +872,7 @@ public: // the cost may be divided accros multiple cells cost = cost / (dtype.TileWidth * dtype.TileWidth); - if (cost < 1) { - cost = 1; - } + cost = std::max(cost, 1); // Removed Unit's are in bunkers int d; diff --git a/src/unit/upgrade.cpp b/src/unit/upgrade.cpp index 4c7e62fab..53e6292d1 100644 --- a/src/unit/upgrade.cpp +++ b/src/unit/upgrade.cpp @@ -582,15 +582,8 @@ static void ApplyUpgradeModifier(CPlayer &player, const CUpgradeModifier *um) stat.Variables[j].Increase += um->Modifier.Variables[j].Increase; } - if (stat.Variables[j].Value < 0) { - stat.Variables[j].Value = 0; - } - if (stat.Variables[j].Max < 0) { - stat.Variables[j].Max = 0; - } - if (stat.Variables[j].Value > stat.Variables[j].Max) { - stat.Variables[j].Value = stat.Variables[j].Max; - } + stat.Variables[j].Max = std::max(stat.Variables[j].Max, 0); + clamp(&stat.Variables[j].Value, 0, stat.Variables[j].Max); } // And now modify ingame units @@ -614,16 +607,10 @@ static void ApplyUpgradeModifier(CPlayer &player, const CUpgradeModifier *um) unit.Variable[j].Increase += um->Modifier.Variables[j].Increase; } - if (unit.Variable[j].Value < 0) { - unit.Variable[j].Value = 0; - } unit.Variable[j].Max += um->Modifier.Variables[j].Max; - if (unit.Variable[j].Max < 0) { - unit.Variable[j].Max = 0; - } - if (unit.Variable[j].Value > unit.Variable[j].Max) { - unit.Variable[j].Value = unit.Variable[j].Max; - } + unit.Variable[j].Max = std::max(unit.Variable[j].Max, 0); + + clamp(&unit.Variable[j].Value, 0, unit.Variable[j].Max); } } } diff --git a/src/video/font.cpp b/src/video/font.cpp index 50181cbb9..77ba75ea0 100644 --- a/src/video/font.cpp +++ b/src/video/font.cpp @@ -805,9 +805,7 @@ void CFont::MeasureWidths() for (; sp < lp; --lp) { if (*lp != ckey && *lp != 7) { - if (lp - sp > CharWidth[y]) { // max width - CharWidth[y] = lp - sp; - } + CharWidth[y] = std::max<char>(CharWidth[y], lp - sp); } } sp += G->Surface->pitch; diff --git a/src/video/graphic.cpp b/src/video/graphic.cpp index 0f43cd926..cba201dff 100644 --- a/src/video/graphic.cpp +++ b/src/video/graphic.cpp @@ -645,7 +645,7 @@ void CGraphic::GenFramesMap() Assert(Width != 0); Assert(Height != 0); - if (frame_map) { delete[] frame_map; } + delete[] frame_map; frame_map = new frame_pos_t[NumFrames]; @@ -818,10 +818,8 @@ void CGraphic::Free(CGraphic *g) #endif { FreeSurface(&g->SurfaceFlip); - if (g->frameFlip_map) { - delete[] g->frameFlip_map; - g->frameFlip_map = NULL; - } + delete[] g->frameFlip_map; + g->frameFlip_map = NULL; } if (!g->HashFile.empty()) { @@ -979,7 +977,7 @@ void CGraphic::Flip() SDL_UnlockSurface(Surface); SDL_UnlockSurface(s); - if (frameFlip_map) { delete[] frameFlip_map; } + delete[] frameFlip_map; frameFlip_map = new frame_pos_t[NumFrames]; @@ -1267,10 +1265,8 @@ void CGraphic::Resize(int w, int h) FreeSurface(&Surface); Surface = NULL; } - if (frame_map) { - delete[] frame_map; - frame_map = NULL; - } + delete[] frame_map; + frame_map = NULL; #if defined(USE_OPENGL) || defined(USE_GLES) if (!UseOpenGL) #endif @@ -1279,10 +1275,8 @@ void CGraphic::Resize(int w, int h) FreeSurface(&SurfaceFlip); SurfaceFlip = NULL; } - if (frameFlip_map) { - delete[] frameFlip_map; - frameFlip_map = NULL; - } + delete[] frameFlip_map; + frameFlip_map = NULL; } #if defined(USE_OPENGL) || defined(USE_GLES)