Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Keep track of all mobs that drop an item #8049

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
+108 −76
Open

Keep track of all mobs that drop an item #8049

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
87360db
Keep track of all mobs that drop an item
vstumpf Jul 20, 2023
e324620
Clean up implementation
vstumpf Dec 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 22 additions & 19 deletions src/map/atcommand.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8368,31 +8368,34 @@ ACMD_FUNC(whodrops)
sprintf(atcmd_output, msg_txt(sd,1285), item_db.create_item_link( id ).c_str(), id->nameid); // Item: '%s' (ID:%u)
clif_displaymessage(fd, atcmd_output);

if (id->mob[0].chance == 0) {
if (id->mobs.empty()) {
strcpy(atcmd_output, msg_txt(sd,1286)); // - Item is not dropped by mobs.
clif_displaymessage(fd, atcmd_output);
} else {
sprintf(atcmd_output, msg_txt(sd,1287), MAX_SEARCH); // - Common mobs with highest drop chance (only max %d are listed):
clif_displaymessage(fd, atcmd_output);
return 0;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return 0;
continue;

It can be more than one item, so sadly we cannot cancel early.

Copy link
Member Author

Choose a reason for hiding this comment

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

oops, you're right

}
sprintf(atcmd_output, msg_txt(sd,1287), MAX_SEARCH); // - Common mobs with highest drop chance (only max %d are listed):
clif_displaymessage(fd, atcmd_output);

for (uint16 j=0; j < MAX_SEARCH && id->mob[j].chance > 0; j++)
{
int dropchance = id->mob[j].chance;
std::shared_ptr<s_mob_db> mob = mob_db.find(id->mob[j].id);
if(!mob) continue;
// NB: We don't const-ref this because we modify dropchance, so let's just make a copy
Copy link
Member

Choose a reason for hiding this comment

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

Wouldnt it be more performant to const-ref this and just have a separate variable for the "real" droprate calculation?

Copy link
Member Author

Choose a reason for hiding this comment

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

ehh, two 32bit copy or a 64bit address with a 32bit copy, not sure which is better

int count = MAX_SEARCH;
for (auto [dropchance, mob_id] : id->mobs) {
std::shared_ptr<s_mob_db> mob = mob_db.find(mob_id);
if (!mob)
continue;

#ifdef RENEWAL_DROP
if( battle_config.atcommand_mobinfo_type ) {
dropchance = dropchance * pc_level_penalty_mod( sd, PENALTY_DROP, mob ) / 100;
if (dropchance <= 0 && !battle_config.drop_rate0item)
dropchance = 1;
}
#endif
if (pc_isvip(sd)) // Display item rate increase for VIP
dropchance += (dropchance * battle_config.vip_drop_increase) / 100;
sprintf(atcmd_output, "- %s (%d): %02.02f%%", mob->jname.c_str(), id->mob[j].id, dropchance/100.);
clif_displaymessage(fd, atcmd_output);
if (battle_config.atcommand_mobinfo_type) {
dropchance = dropchance * pc_level_penalty_mod(sd, PENALTY_DROP, mob) / 100;
if (dropchance <= 0 && !battle_config.drop_rate0item)
dropchance = 1;
}
#endif
if (pc_isvip(sd)) // Display item rate increase for VIP
dropchance += (dropchance * battle_config.vip_drop_increase) / 100;
sprintf(atcmd_output, "- %s (%d): %02.02f%%", mob->jname.c_str(), mob_id, dropchance/100.);
clif_displaymessage(fd, atcmd_output);
if (--count <= 0)
break;
}
}
return 0;
Expand Down
9 changes: 5 additions & 4 deletions src/map/itemdb.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2259,10 +2259,11 @@ struct item_data
// some script commands should be revised as well...
uint64 class_base[3]; //Specifies if the base can wear this item (split in 3 indexes per type: 1-1, 2-1, 2-2)
uint16 class_upper; //Specifies if the class-type can equip it (See e_item_job)
struct {
int chance;
int id;
} mob[MAX_SEARCH]; //Holds the mobs that have the highest drop rate for this item. [Skotlex]
struct drop_chance {
Copy link
Member

Choose a reason for hiding this comment

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

Would be nice to move this outside and follow the naming standards.

Copy link
Member Author

Choose a reason for hiding this comment

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

What's the naming standards?
This struct only has a meaning inside item_data, so i made it a struct inside.

uint32 chance;
uint32 mob_id;
};
std::vector<drop_chance> mobs; // Sorted vector that holds the mobs that have the highest drop rate for this item.
Copy link
Member

Choose a reason for hiding this comment

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

This vector has no limit now anymore.

struct script_code *script; //Default script for everything.
struct script_code *equip_script; //Script executed once when equipping.
struct script_code *unequip_script;//Script executed once when unequipping.
Expand Down
46 changes: 18 additions & 28 deletions src/map/mob.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6284,20 +6284,11 @@ static void mob_drop_ratio_adjust(void){
id->maxchance = rate; // item has bigger drop chance or sold in shops
}

for( k = 0; k < MAX_SEARCH; k++ ){
if( id->mob[k].chance <= rate ){
break;
}
}

if( k != MAX_SEARCH ){
if( id->mob[k].id != mob_id ){
memmove( &id->mob[k+1], &id->mob[k], (MAX_SEARCH-k-1)*sizeof(id->mob[0]) );
}
item_data::drop_chance drop{static_cast<uint32>(rate), pair.second->id};

id->mob[k].chance = rate;
id->mob[k].id = mob_id;
}
id->mobs.insert(std::lower_bound(id->mobs.begin(), id->mobs.end(), drop, [](const auto &drop, const auto &it) {
return drop.chance > it.chance;
}), drop);
}

mob->dropitem[j].rate = rate;
Expand Down Expand Up @@ -6609,30 +6600,29 @@ void mob_db_load(bool is_reload){
*/
void mob_reload_itemmob_data(void) {
for( auto const &pair : mob_db ){
int d, k;

if( mob_is_clone( pair.first ) ){
continue;
}

for(d = 0; d < MAX_MOB_DROP_TOTAL; d++) {
struct item_data *id;
for(int d = 0; d < MAX_MOB_DROP_TOTAL; d++) {
if( !pair.second->dropitem[d].nameid )
continue;
id = itemdb_search(pair.second->dropitem[d].nameid);

for (k = 0; k < MAX_SEARCH; k++) {
if (id->mob[k].chance <= pair.second->dropitem[d].rate)
break;
}

if (k == MAX_SEARCH)
struct item_data *id = itemdb_search(pair.second->dropitem[d].nameid);
if (!id)
continue;

if (id->mob[k].id != pair.first)
memmove(&id->mob[k+1], &id->mob[k], (MAX_SEARCH-k-1)*sizeof(id->mob[0]));
id->mob[k].chance = pair.second->dropitem[d].rate;
id->mob[k].id = pair.first;
item_data::drop_chance drop{pair.second->dropitem[d].rate, pair.second->id};

id->mobs.insert(
std::lower_bound(
id->mobs.begin(),
id->mobs.end(),
drop,
[](const auto &drop, const auto &it) {
return drop.chance < it.chance;
Copy link
Member

Choose a reason for hiding this comment

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

Why is this the only place where we use <?
All others use >.

}),
drop);
}
}
}
Expand Down
58 changes: 40 additions & 18 deletions src/map/script.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18239,7 +18239,16 @@ BUILDIN_FUNC(addmonsterdrop)
mob->dropitem[c].rate = rate;
mob->dropitem[c].steal_protected = steal_protected > 0;
mob->dropitem[c].randomopt_group = group;
mob_reload_itemmob_data(); // Reload the mob search data stored in the item_data
item_data::drop_chance drop{static_cast<uint32>(rate), mob->id};
itm->mobs.insert(
std::lower_bound(
itm->mobs.begin(),
itm->mobs.end(),
drop,
[](const auto &drop, const auto &it) {
return drop.chance > it.chance;
}),
drop);

script_pushint(st, true);
return SCRIPT_CMD_SUCCESS;
Expand Down Expand Up @@ -18270,25 +18279,38 @@ BUILDIN_FUNC(delmonsterdrop)
return SCRIPT_CMD_FAILURE;
}

if(mob) { //We got a valid monster, check for item drop on monster
unsigned char i;
for(i = 0; i < MAX_MOB_DROP_TOTAL; i++) {
if(mob->dropitem[i].nameid == item_id) {
mob->dropitem[i].nameid = 0;
mob->dropitem[i].rate = 0;
mob->dropitem[i].steal_protected = false;
mob->dropitem[i].randomopt_group = 0;
mob_reload_itemmob_data(); // Reload the mob search data stored in the item_data
script_pushint(st,1);
return SCRIPT_CMD_SUCCESS;
}
}
//No drop on that monster
script_pushint(st,0);
} else {
ShowWarning("delmonsterdrop: bad mob id given %d\n",script_getnum(st,2));
if (!mob) {
if (script_isstring(st, 2))
ShowWarning("delmonsterdrop: bad mob name given %s\n", script_getstr(st, 2));
else
ShowWarning("delmonsterdrop: bad mob id given %d\n", script_getnum(st, 2));
return SCRIPT_CMD_FAILURE;
}
Copy link
Member

Choose a reason for hiding this comment

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

Can we combine that with the if/else right at the start?
The item db lookup is kind of useless, if we do not have a valid mob.
Also please change it to ShowError


unsigned char i;
Copy link
Member

Choose a reason for hiding this comment

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

Please change this to be the loop variable.

for(i = 0; i < MAX_MOB_DROP_TOTAL; i++) {
if(mob->dropitem[i].nameid != item_id) {
continue;
}
mob->dropitem[i].nameid = 0;
mob->dropitem[i].rate = 0;
mob->dropitem[i].steal_protected = false;
mob->dropitem[i].randomopt_group = 0;

std::shared_ptr<item_data> itm = item_db.find(item_id);
Copy link
Member

Choose a reason for hiding this comment

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

There is already the following call at the start:
if(!item_db.exists(item_id)){

Please rewrite it and only look it up once at the start of the function.

if (itm) {
auto it = std::find_if(itm->mobs.begin(), itm->mobs.end(), [&mob](const auto &drop) {
return drop.mob_id == mob->id;
});
if (it != itm->mobs.end())
itm->mobs.erase(it);
Copy link
Member

Choose a reason for hiding this comment

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

Why dont we std::remove_if here?

Copy link
Member Author

Choose a reason for hiding this comment

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

we only want to remove one drop, remove_if moves all to the end, so i went with find

}

script_pushint(st,1);
return SCRIPT_CMD_SUCCESS;
Comment on lines +18299 to +18300
Copy link
Member

Choose a reason for hiding this comment

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

I think this was broken sometime in the past, since we support monsters dropping the same item id multiple times now.

}
//No drop on that monster
script_pushint(st,0);
return SCRIPT_CMD_SUCCESS;
}

Expand Down
Loading