summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorHaru <haru@dotalux.com>2017-11-05 21:13:14 +0100
committerHaru <haru@dotalux.com>2017-11-05 21:13:14 +0100
commit3edcd549cee863ecc8a2a96dfb5bbb56d435d97a (patch)
treec08ceaa38363344110bb0d2b7b4e556e3a88c436
parent09d5c20572424e7a92dff5cf560001278c6ffaa0 (diff)
downloadhercules-3edcd549cee863ecc8a2a96dfb5bbb56d435d97a.tar.gz
hercules-3edcd549cee863ecc8a2a96dfb5bbb56d435d97a.tar.bz2
hercules-3edcd549cee863ecc8a2a96dfb5bbb56d435d97a.tar.xz
hercules-3edcd549cee863ecc8a2a96dfb5bbb56d435d97a.zip
Fix/clarify various incorrect modulo operations
Most relevantly, `skill_lv%11 - 1` is not a valid skill item requirement index, since it can return -1 depending on the skill_lv. It was replaced with `(skill_lv - 1) % MAX_SKILL_ITEM_REQUIRE`, which always returns a value in the 0 ~ MAX_SKILL_ITEM_REQUIRE range. Signed-off-by: Haru <haru@dotalux.com>
-rw-r--r--src/map/elemental.c2
-rw-r--r--src/map/mercenary.c2
-rw-r--r--src/map/script.c2
-rw-r--r--src/map/skill.c47
4 files changed, 27 insertions, 26 deletions
diff --git a/src/map/elemental.c b/src/map/elemental.c
index ae1fda0a2..cf1d485e1 100644
--- a/src/map/elemental.c
+++ b/src/map/elemental.c
@@ -885,7 +885,7 @@ int read_elementaldb(void) {
estatus->race = atoi(str[20]);
ele = atoi(str[21]);
- estatus->def_ele = ele%10;
+ estatus->def_ele = ele % ELE_MAX;
estatus->ele_lv = ele/20;
if( estatus->def_ele >= ELE_MAX ) {
ShowWarning("Elemental %d has invalid element type %d (max element is %d)\n", db->class_, estatus->def_ele, ELE_MAX - 1);
diff --git a/src/map/mercenary.c b/src/map/mercenary.c
index f5d3fe11c..c4b692008 100644
--- a/src/map/mercenary.c
+++ b/src/map/mercenary.c
@@ -465,7 +465,7 @@ bool read_mercenarydb_sub(char* str[], int columns, int current) {
mstatus->race = atoi(str[20]);
ele = atoi(str[21]);
- mstatus->def_ele = ele%10;
+ mstatus->def_ele = ele % ELE_MAX;
mstatus->ele_lv = ele/20;
if( mstatus->def_ele >= ELE_MAX ) {
ShowWarning("Mercenary %d has invalid element type %d (max element is %d)\n", db->class_, mstatus->def_ele, ELE_MAX - 1);
diff --git a/src/map/script.c b/src/map/script.c
index f72b54a0d..a836501f7 100644
--- a/src/map/script.c
+++ b/src/map/script.c
@@ -12190,7 +12190,7 @@ BUILDIN(homunculus_mutate)
if (script_hasdata(st,2))
homun_id = script_getnum(st,2);
else
- homun_id = 6048 + (rnd() % 4);
+ homun_id = HOMID_EIRA + (rnd() % 4);
m_class = homun->class2type(sd->hd->homunculus.class_);
m_id = homun->class2type(homun_id);
diff --git a/src/map/skill.c b/src/map/skill.c
index 710b4d403..1b7d40e83 100644
--- a/src/map/skill.c
+++ b/src/map/skill.c
@@ -7414,7 +7414,7 @@ int skill_castend_nodamage_id(struct block_list *src, struct block_list *bl, uin
}
if( sd ) {
int bonus = 100, potion = min(500+skill_lv,505);
- int item_idx = skill_lv%11 - 1;
+ int item_idx = (skill_lv - 1) % MAX_SKILL_ITEM_REQUIRE;
int item_id = skill->get_itemid(skill_id, item_idx);
int inventory_idx = pc->search_inventory(sd, item_id);
if (inventory_idx == INDEX_NOT_FOUND || item_id <= 0) {
@@ -7757,7 +7757,7 @@ int skill_castend_nodamage_id(struct block_list *src, struct block_list *bl, uin
maxlv = skill_lv - 4;
}
else if(skill_lv >=2) {
- int i = rnd()%3;
+ int i = rnd() % ARRAYLENGTH(spellarray);
spellid = spellarray[i];
maxlv = skill_lv - 1;
}
@@ -8300,7 +8300,7 @@ int skill_castend_nodamage_id(struct block_list *src, struct block_list *bl, uin
clif->damage(src,bl,0,0,1000,0,BDT_NORMAL,0);
if( !status->isdead(bl) ) {
int where[] = { EQP_ARMOR, EQP_SHIELD, EQP_HELM, EQP_SHOES, EQP_GARMENT };
- skill->break_equip(bl, where[rnd()%5], 10000, BCT_ENEMY);
+ skill->break_equip(bl, where[rnd() % ARRAYLENGTH(where)], 10000, BCT_ENEMY);
}
}
break;
@@ -8321,7 +8321,7 @@ int skill_castend_nodamage_id(struct block_list *src, struct block_list *bl, uin
case 7: // stop freeze or stoned
{
enum sc_type sc[] = { SC_STOP, SC_FREEZE, SC_STONE };
- sc_start(src,bl,sc[rnd()%3],100,skill_lv,skill->get_time2(skill_id,skill_lv));
+ sc_start(src,bl,sc[rnd() % ARRAYLENGTH(sc)],100,skill_lv,skill->get_time2(skill_id,skill_lv));
}
break;
case 8: // curse coma and poison
@@ -11211,7 +11211,7 @@ int skill_castend_pos2(struct block_list* src, int x, int y, uint16 skill_id, ui
// Slim Pitcher [Celest]
case CR_SLIMPITCHER:
if (sd) {
- int item_idx = skill_lv%11 - 1;
+ int item_idx = (skill_lv - 1) % MAX_SKILL_ITEM_REQUIRE;
int item_id = skill->get_itemid(skill_id, item_idx);
int inventory_idx = pc->search_inventory(sd, item_id);
int bonus;
@@ -11244,7 +11244,7 @@ int skill_castend_pos2(struct block_list* src, int x, int y, uint16 skill_id, ui
skill->castend_nodamage_id);
}
} else {
- int item_idx = skill_lv%11 - 1;
+ int item_idx = (skill_lv - 1) % MAX_SKILL_ITEM_REQUIRE;
int item_id = skill->get_itemid(skill_id, item_idx);
struct item_data *item = itemdb->search(item_id);
int bonus;
@@ -12013,27 +12013,27 @@ struct skill_unit_group* skill_unitsetting(struct block_list *src, uint16 skill_
int element[5]={ELE_WIND,ELE_DARK,ELE_POISON,ELE_WATER,ELE_FIRE};
val1 = st->rhw.ele;
- if (!val1)
- val1=element[rnd()%5];
+ if (val1 == ELE_NEUTRAL)
+ val1 = element[rnd() % ARRAYLENGTH(element)];
- switch (val1)
- {
+ switch (val1) {
case ELE_FIRE:
- subunt++;
- FALLTHROUGH
+ subunt = 4;
+ break;
case ELE_WATER:
- subunt++;
- FALLTHROUGH
+ subunt = 3;
+ break;
case ELE_POISON:
- subunt++;
- FALLTHROUGH
+ subunt = 2;
+ break;
case ELE_DARK:
- subunt++;
- FALLTHROUGH
+ subunt = 1;
+ break;
case ELE_WIND:
+ subunt = 0;
break;
default:
- subunt=rnd()%5;
+ subunt = rnd() % 5;
break;
}
@@ -15201,7 +15201,8 @@ struct skill_condition skill_get_requirement(struct map_session_data* sd, uint16
}
for( i = 0; i < MAX_SKILL_ITEM_REQUIRE; i++ ) {
- if( (skill_id == AM_POTIONPITCHER || skill_id == CR_SLIMPITCHER || skill_id == CR_CULTIVATION) && i != skill_lv%11 - 1 )
+ int item_idx = (skill_lv - 1) % MAX_SKILL_ITEM_REQUIRE;
+ if ((skill_id == AM_POTIONPITCHER || skill_id == CR_SLIMPITCHER || skill_id == CR_CULTIVATION) && i != item_idx)
continue;
switch( skill_id ) {
@@ -18097,7 +18098,7 @@ int skill_produce_mix(struct map_session_data *sd, uint16 skill_id, int nameid,
int difficulty = 30 + rnd()%120; // Random number between (30 ~ 150)
make_per = sd->status.job_level / 4 + st->luk / 2 + st->dex / 3; // (Caster?s Job Level / 4) + (Caster?s LUK / 2) + (Caster?s DEX / 3)
- qty = ~(5 + rnd()%5) + 1;
+ qty = ~(5 + rnd()%5) + 1; // FIXME[Haru]: This smells, if anyone knows the intent, please rewrite the expression in a more clear form.
switch(nameid){// difficulty factor
case ITEMID_APPLE_BOMB:
@@ -18133,7 +18134,7 @@ int skill_produce_mix(struct map_session_data *sd, uint16 skill_id, int nameid,
qty = 5;
if( qty < 0 || (skill_lv == 1 && make_per < difficulty)){
- qty = ~qty + 1;
+ qty = ~qty + 1; // FIXME[Haru]: This smells. If anyone knows the intent, please rewrite the expression in a more clear form.
make_per = 0;
}else
make_per = 10000;
@@ -18390,7 +18391,7 @@ int skill_produce_mix(struct map_session_data *sd, uint16 skill_id, int nameid,
int rate = rnd()%500;
memset(&tmp_item,0,sizeof(tmp_item));
if( rate < 50) i = 4;
- else if( rate < 100) i = 2+rnd()%1;
+ else if( rate < 100) i = 2+rnd()%1; // FIXME[Haru]: This '%1' is certainly not intended. If anyone knows the purpose, please rewrite this code.
else if( rate < 250 ) i = 1;
else if( rate < 500 ) i = 0;
tmp_item.nameid = compensation[i];