Skip to content

Conversation

@guilherme-gm
Copy link
Member

@guilherme-gm guilherme-gm commented Oct 25, 2018

Pull Request Prelude

Changes Proposed

Change 1 : Support to random option drop

This PR adds support for dropping items that contains Random Options in them (used by some official features -- e.g: Crimsom Weapons).

A new database file was included: db/option_drop_group.conf that allows users to create groups of options with their rates.

These groups are then linked on mob_db.conf drop entries, I have changed a bit the Drops to provide an alternate syntax: AegisName: (chance, "GROUP_NAME").

I didn't like how I coded the part that picks an option for a slot (I'll make a comment in this line), but I couldn't think of another way until now, so I'm open fo suggestions on that.

Change 2 : Moved mobs drop data to mob_drop structure (as suggested in PR review)

Issues addressed:
I think there isn't one.

Thanks to @Ridley8819 for explaining me how this was supposed to work, @Asheraf and @MishimaHaruna for giving suggestions on the structure.

@HerculesWSAPI
Copy link
Contributor

This change is Reviewable

src/map/mob.c Outdated

// count avoids a too long loop that would cause lag. if after 10 full iterations it still fails
// it'll stop in one random index in the next one.
count = 10 * options->options[i].option_count + (rnd() % options->options[i].option_count);
Copy link
Member Author

Choose a reason for hiding this comment

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

I didn't like this count, I think it kind of breaks the idea of rates, but I couldn't really think of a better way. One idea was to make something like we do with boxes, where we put how many times that item appears in the set.

@Helianthella Helianthella added component:databases Affecting the databases available in the db/ folder component:mechanics Affecting the game mechanics in general codereview:needsedits Some edits have been requested before the pull request can be accepted labels Nov 8, 2018
@Asheraf Asheraf added this to the Release v2018.11.18 milestone Nov 8, 2018
@tlacson7
Copy link

tlacson7 commented Nov 9, 2018

CC HPMHooking.c (CHAR) PLUGIN HPMHooking_char CC HPMHooking.c (LOGIN) PLUGIN HPMHooking_login CC HPMHooking.c (MAP) In file included from HPMHooking.c:228:0: HPMHooking/HPMHooking_map.Hooks.inc: In function ‘HP_mob_setdropitem’: HPMHooking/HPMHooking_map.Hooks.inc:51236:3: warning: passing argument 2 of ‘HPMHooks.source.mob.setdropitem’ makes pointer from integer without a cast [enabled by default] retVal___ = HPMHooks.source.mob.setdropitem(nameid, qty, data); ^ HPMHooking/HPMHooking_map.Hooks.inc:51236:3: note: expected ‘struct mob_drop_option *’ but argument is of type ‘int’ HPMHooking/HPMHooking_map.Hooks.inc:51236:3: warning: passing argument 3 of ‘HPMHooks.source.mob.setdropitem’ makes integer from pointer without a cast [enabled by default] HPMHooking/HPMHooking_map.Hooks.inc:51236:3: note: expected ‘int’ but argument is of type ‘struct item_data *’ HPMHooking/HPMHooking_map.Hooks.inc:51236:3: error: too few arguments to function ‘HPMHooks.source.mob.setdropitem’ make[1]: *** [../../plugins/HPMHooking_map.so] Error 1

@4144
Copy link
Contributor

4144 commented Nov 9, 2018

@tlacson7 this is probably because hpm hooks not updated.
open tools/HPMHookGen and run make after it will update hpm hooks, and you can try to build it

@guilherme-gm
Copy link
Member Author

Hello everybody, sorry for the delay.
Recently I was talking to @Ridley8819 and we noticed that this structure will make mob_db file even bigger than it already is, so I'm thinking on moving the option settings to a separate (option_drop_group.conf) file, this not only would reduce the file size when options get added, but also allows us to reuse the same group of options. The structure would be like that:

mob_db.conf:

Drops: {
    AegisName: chance
    // or
    AegisName: [Drop Rate, Option group]
}

// Example:
Drops: {
    Apple: 7000
    Knife: [1000, OPT_GROUP_SOMEITEMS] // So knife will use OPT_GROUP_SOMEITEMS from option_drop_group
}

option_drop_group: (structure that is in this PR already)

{
    Group: "Group_Name"
    Options: (            options per slot
        {
            Rate: chance of option 1  (int)

            // Option possibilites for this slot
            // min/max value : int, defaults to 0
            // rate : int, 100 = 1% if not set, will be 100%/number of possibiltiies
            OptionName1: value
            // or
            OptionName1: [min value, max value]
            // or
            OptionName1: [min value, max value, rate]
            // ...  (up to MAX_MOB_DROP_OPTIONS)
        },
        // ... (up to MAX_ITEM_OPTION)
    )
}

// Example:
{
    Group: "OPT_GROUP_SOMEITEMS"
    Options: (        // options per slot
        {
            Rate: 5000
            RACE_TOLERACE_DEVIL: 10000
        },
    )
}

what you guys think?

@AnnieRuru
Copy link
Contributor

yes having a separate group structure is better

just want to make sure ... this thing is entirely custom ... unofficial right ?
like .... we also don't support item drop with equipment refined ... from source

@guilherme-gm
Copy link
Member Author

Official servers does have drops with random options (e.g. Crimsom Weapons), but the implementation itself is solely based on the information I could find (which isn't much). So I made it as customizable as possible to try to cover whatever is needed once those items are implemented.

@Asheraf
Copy link
Contributor

Asheraf commented Feb 20, 2019

@guilherme-gm Groups would be nice :)

@AnnieRuru
Copy link
Contributor

oh ok, http://www.playragnarok.com/news/updatedetail.aspx?id=280&p=1
its under Episode 15.2

asdf

Ps: I actually thought it has something to do with makeitem3 ... seems I was wrong
btw all makeitem2 in hercules/npc are slash-out

@guilherme-gm
Copy link
Member Author

PR updated, I did some major changes to support the extra db file and changed a bit how it is stored (I believe this way will save more memory when compared to the old approach), I tried to remember all the changes requested while fixing it and hopefully I haven't forgot anything. Thanks for your reviews :)

@Asheraf Asheraf added status:code-review Awaiting code review and removed codereview:needsedits Some edits have been requested before the pull request can be accepted labels Feb 24, 2019
@4144
Copy link
Contributor

4144 commented Feb 24, 2019

according ci, server crashing with memory manager

@guilherme-gm guilherme-gm force-pushed the 201810-randomopt-drop branch 2 times, most recently from 0706ed7 to d176d28 Compare February 25, 2019 19:47
@guilherme-gm guilherme-gm force-pushed the 201810-randomopt-drop branch from d176d28 to a1ca68e Compare March 3, 2019 13:08
Copy link
Contributor

@AnnieRuru AnnieRuru left a comment

Choose a reason for hiding this comment

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

my review is done.
technical C language structure, ... not really my soup of taste

@guilherme-gm guilherme-gm force-pushed the 201810-randomopt-drop branch 2 times, most recently from 8e2c390 to c3c1f7a Compare March 6, 2019 14:36
@guilherme-gm guilherme-gm force-pushed the 201810-randomopt-drop branch from c3c1f7a to 8289c07 Compare April 2, 2019 00:44
@guilherme-gm
Copy link
Member Author

@hemagx @4144 @Asheraf can you review it again? thanks!

@guilherme-gm guilherme-gm force-pushed the 201810-randomopt-drop branch 2 times, most recently from 52dea5d to 44462af Compare April 7, 2019 18:52
@guilherme-gm guilherme-gm force-pushed the 201810-randomopt-drop branch from 44462af to 7810113 Compare April 20, 2019 18:46
@MishimaHaruna MishimaHaruna merged commit 78f9922 into HerculesWS:master May 5, 2019
@guilherme-gm guilherme-gm mentioned this pull request May 6, 2019
3 tasks
@guilherme-gm guilherme-gm deleted the 201810-randomopt-drop branch February 24, 2023 15:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component:databases Affecting the databases available in the db/ folder component:mechanics Affecting the game mechanics in general status:code-review Awaiting code review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants