herculesws / hercules Goto Github PK

timer.c::do_timer

        if( timer_data[tid].func ) {
            if( diff < -1000 )
                // timer was delayed for more than 1 second, use current tick instead
                timer_data[tid].func(tid, tick, timer_data[tid].id, timer_data[tid].data);
            else
                timer_data[tid].func(tid, timer_data[tid].tick, timer_data[tid].id, timer_data[tid].data);
        }

timer.h

typedef int (*TimerFunc)(int tid, unsigned int tick, int id, intptr_t data);

Code has been the same since eAthena and as I understand this problem is this old, lies at how npc functions try to override if got 2 with same name

npc_parse_function

    func_db = script_get_userfunc_db();
    oldscript = (struct script_code*)strdb_put(func_db, w3, script);
    if( oldscript != NULL )
    {
        ShowInfo("npc_parse_function: Overwriting user function [%s] (%s:%d)\n", w3, filepath, strline(buffer,start-buffer));
        script_free_vars(&oldscript->script_vars);
        aFree(oldscript->script_buf);
        aFree(oldscript);
    }

ours looks sightly different but the design is the same and remains broken

    func_db = script->userfunc_db;
    if (func_db->put(func_db, DB->str2key(w3), DB->ptr2data(scriptroot), &old_data))
    {
        struct script_code *oldscript = (struct script_code*)DB->data2ptr(&old_data);
        ShowInfo("npc_parse_function: Overwriting user function [%s] (%s:%d)\n", w3, filepath, strline(buffer,start-buffer));
        script->free_vars(oldscript->script_vars);
        aFree(oldscript->script_buf);
        aFree(oldscript);
    }

Pretty much if you have a function sleeping/timer'd while this is performed (e.g. by doing @loadnpc on a file already loaded that has a function) the previous pointer stuff screw up and ends up crashing in run_script_main ( claims st->script->script_buf is empty aka 0xfdfdfdfdfd'd by the memory manager ).

clif.c

void clif_maptypeproperty2(struct block_list *bl,enum send_target t) {
    struct packet_maptypeproperty2 p;

    p.PacketType = maptypeproperty2Type;
    p.type = 0x28;
    p.flag.usecart = 1;
    p.flag.party = 1;
    p.flag.guild = 1;
    p.flag.siege = map_flag_gvg2(bl->m) ? 1: 0;
    p.flag.mineffect = 1;
    p.flag.nolockon = 0;
    p.flag.countpk = map[bl->m].flag.pvp ? 1 : 0;
    p.flag.nopartyformation = 0;
    p.flag.noitemconsumption = 0;
    p.flag.summonstarmiracle = 0;
    p.flag.bg = map[bl->m].flag.battleground ? 1 : 0;

    clif->send(&p,sizeof(p),bl,t);
}

as its visible the ones with 0 are missing mapflags, some with a straight 1 could be made into mapflags too (e.g. usecart)

No Implementation at all, missing.

Fully Integrated

No Integration at all, even the buildin_ is missing the latest update that changed return values from 1/0 to false/true

from conf/login-server.conf:

// Log Filename. All operations received by the server are logged in this file. 
login_log_filename: log/login.log

Fully Integrated.

Fully Integrated.

Fully Integrated

Incomplete, little to no integration (1 member in the interface)

e.g. conf/atcommand.conf

Partially Integrated, Incomplete.

The tombstone of Lord Of Death in niflheim map is not working. I already set the monster.conf in mvp_tomb_enabled to yes. Please confirm. Thank you

After using the command @skillsall or @skillall the skill points is not working on Union Skill of the Star Gladiator. I already try manual adding points but still not working. Im using latest version of hercules emulator. Sorry my bad english.

No Integration at all, missing.

• Enable party/guild support (existent but disabled) and add the splitting algorithm, as Shikazu drafted ( staff link; http://hercules.ws/board/topic/1192-bg-queue-splitting-teams/ )
• in battlegrounds.c bg_canqueue gdelay_var and arena->delay_var checks are checking against time() instead of time()+proper_setting
• script->hq[arena->queue_id].items is apparently not being cleared (after one bg takes place users get their position as if the previous queue was still in place), even though it is thrown in queue delete (might be a queue problem instead of the bg)
• the feature should save your position when you're warped into the bg and warp you back when it ends ( no support for this is currently present )
• when you're in a queue the client has a window displaying your position and a 'leave' button (beside the windows close button), this button is currently doing nothing while it should get you out of the queue.

( taken from http://hercules.ws/board/topic/1302-bg-queue-debug/ )

Fully Integrated.

Fully Integrated.

GreenBox has remade the system from the ground up a couple months ago but the description in the top of the file is still the same as previous and some are off (I'm not sure about the others), for example:

 *  - A  manager will only auto-destroy when all of its instances are        *
 *    destroyed so memory will usually only be recovered near the end.       *

This is no longer true

Euphy just committed this: rathena/rathena@13b281b
If a script dev could go over it and check if it's compatible with ours that'd be great.

Fully Integrated.

Currently in Hercules there are 2 ways to store user passwords in database:

• unsalted one-round MD5 hash (bad)
• plain text (arguably unacceptable)

I've done little research on storing passwords in safe manner these days, and it looks like bcrypt is generally recommended and widespread solution. It also has ready to use implementations in many languages, most importantly PHP and C.

I think we should provide a way to store passwords in a way that is considered safe. MD5 maybe was good enough several years ago, but with increasing computing power it no longer is.

Fully Integrated.

No Integration at all, missing.

Fully Integrated.

Currently no integration at all

• As Xgear suggested: we could differentiate between an exported copy that lacks any .git or .svn folders, and a valid working copy where the version detection fails for any other reason (our methods to read that info aren't really orthodox.)

• As Gepard suggested, the string 'exported' can be used in case we find out that it's an exported copy.

• The current git HEAD is quite weak, as it only reports the SHA1 of the current refs/remotes/origin/master, which isn't necessarily the currently running version, but it assumes that (those are probably valid for many users, but not necessarily):

  • The current branch pulls from origin/master
  • The user never runs git fetch, but only git pull, and never does so on other branches
  • The user never uses git checkout

  It is also true that, both now and if we fix that to detect the version of the current HEAD, it may lead to a SHA1 that's not part of the official repository, in case the user has committed changes to their own working copy.

• I'd like to add: our method to detect the current version only does so at runtime, and, in case the sources are an older version than the working copy (i.e. ony the scripts are updated, and the sources haven't been recompiled and/or make install has been used - when and if it'll ever be working), it's not necessarily the correct info we want. I'd like to see the version detection in two different places: one at runtime as it is now, and the other in the build script, so that it gets hardcoded as a string in the compiled executable. I can do this last part for the unix build system for both git and svn, but I have no idea about how to add it to the Visual Studio builds.

Gepard proposed turning HCache's 1-byte key (currently 'k') into a define, so its not hardcoded in multiple areas as it currently is

Partially Integrated, incomplete.

Fully Integrated.

Fully integrated.

EDP it's not completed, lack of damage on certain skills. More info on issue #7559

https://github.com/HerculesWS/Hercules/blob/master/npc/quests/mage_solution.txt

File has no header.

Fully Integrated.

No Integration at all, missing.

Fully Integrated.

As pointed out in b543724#commitcomment-3821265 I haven't updated the SQL file to match the item_db (pre-re) changes in b543724.

The current SQL scripts were generated by PhpMyAdmin, which I don't have installed (nor I plan to, for various reasons), If I were to do it with mysqldump, I'd cause an unnecessary amount of line changes due to a different MySQL syntax.

I'm leaving this here as a reminder, for anyone who has the needed software installed and can update the file (@shennetsind?).

Thank you.

Until recently

• 0aee4fd added

#define safesnprintf(buf,sz,fmt,...) strlib->safesnprintf(buf,sz,fmt,##__VA_ARGS__)

• 2f9f7ce added

#define packet(id, size, ...) packetdb_addpacket(id, size, ##__VA_ARGS__, 0xFFFF)

there was no variadic macros in source with the exception for some unused code in nullpo.h:

#if __STDC_VERSION__ >= 199901L
/* C99‚ɑΉž */
#define nullpo_ret_f(t, fmt, ...) \
    if (nullpo_chk_f(NLP_MARK, (void *)(t), (fmt), __VA_ARGS__)) {return(0);}

but as you can see this part was checking for C99 support.

So the question is, are we now just assuming that modern compilers can handle this?

I found this part when I was exploring enum code a bit:

    case SC__STRIPACCESSARY:
        if( sd ) {
            int i = -1;
            if( !(sd->bonus.unstripable_equip&EQI_ACC_L) ) {
                i = sd->equip_index[EQI_ACC_L];
                if( i >= 0 && sd->inventory_data[i] && sd->inventory_data[i]->type == IT_ARMOR )
                    pc->unequipitem(sd,i,3); //L-Accessory
            } if( !(sd->bonus.unstripable_equip&EQI_ACC_R) ) {
                i = sd->equip_index[EQI_ACC_R];
                if( i >= 0 && sd->inventory_data[i] && sd->inventory_data[i]->type == IT_ARMOR )
                    pc->unequipitem(sd,i,3); //R-Accessory
            }
            if( i < 0 )
                return 0;
        }

EQI_* is not a bitmask, so I guess it's used improperly here.

Quick references lookup backs it up as well:

Solution
    C:\Users\gepard\Documents\GitHub\HerculesWS\Hercules\vcproj-10\map-server.vcxproj
        C:\Users\gepard\Documents\GitHub\HerculesWS\Hercules\src\map\status.c
            status_change_start (6755):    if(sd->bonus.unstripable_equip&EQP_WEAPON)
            status_change_start (6772):    if(sd->bonus.unstripable_equip&EQP_SHIELD)
            status_change_start (6784):    if(sd->bonus.unstripable_equip&EQP_ARMOR)
            status_change_start (6796):    if(sd->bonus.unstripable_equip&EQP_HELM)
            status_change_start (6868):    if( !(sd->bonus.unstripable_equip&EQI_ACC_L) ) {
            status_change_start (6872):    } if( !(sd->bonus.unstripable_equip&EQI_ACC_R) ) {
        C:\Users\gepard\Documents\GitHub\HerculesWS\Hercules\src\map\pc.c
            pc_bonus (2649):    sd->bonus.unstripable_equip |= EQP_WEAPON;
            pc_bonus (2654):    sd->bonus.unstripable_equip |= EQP_ARMOR;
            pc_bonus (2658):    sd->bonus.unstripable_equip |= EQP_HELM;
            pc_bonus (2662):    sd->bonus.unstripable_equip |= EQP_SHIELD;
            pc_readparam (7260):    case SP_UNSTRIPABLE_WEAPON: val = (sd->bonus.unstripable_equip&EQP_WEAPON)?1:0; break;
            pc_readparam (7263):    val = (sd->bonus.unstripable_equip&EQP_ARMOR)?1:0;
            pc_readparam (7265):    case SP_UNSTRIPABLE_HELM: val = (sd->bonus.unstripable_equip&EQP_HELM)?1:0; break;
            pc_readparam (7266):    case SP_UNSTRIPABLE_SHIELD: val = (sd->bonus.unstripable_equip&EQP_SHIELD)?1:0; break;
        C:\Users\gepard\Documents\GitHub\HerculesWS\Hercules\src\map\pc.h
            _struct_4296bde2_296_6e (327):    unsigned short unstripable_equip;

It's for the http://hercules.ws/board/tracker/issue-7308-guild-aura-on-ally/ report
reference: battle.h

enum e_battle_check_target { //New definitions [Skotlex]
    BCT_NOONE       =   0x000000,
    BCT_SELF        =   0x010000,
    BCT_ENEMY       =   0x020000,
    BCT_PARTY       =   0x040000,
    BCT_GUILD       =   0x080000,
    BCT_NEUTRAL     =   0x100000,
    BCT_SAMEGUILD   =   0x200000,   // No Guild Allies

    BCT_NOGUILD     =   0x170000,   // This should be (~BCT_GUILD&BCT_ALL)
    BCT_NOPARTY     =   0x1b0000,   // This should be (~BCT_PARTY&BCT_ALL)
    BCT_NOENEMY     =   0x1d0000,   // This should be (~BCT_ENEMY&BCT_ALL)

    BCT_ALL         =   0x1f0000,   // Sum of BCT_NOONE to BCT_NEUTRAL
};

skill.c has this thing

int skill_get_unit_target( uint16 skill_id )       { skill_get (skill_db[skill_id].unit_target&BCT_ALL, skill_id); }

which is singling out BCT_SAMEGUILD, how can we best get around this? as I understand modifying BCT_ALL is out of the question as it'd be inconsistent since sameguild excludes guild allies from the check.

Fully Integrated.

Fully Integrated.

Fully Integrated.

Fully Integrated.

This is something I've been thinking about for long time now.

• proper separation of battle and skill code into their own modules (eg int battle_calc_skillratio is placed in battle.c now, instead of skill.c)
• splitting code into smaller functions responsible of performing exactly one task (ideally) for improved readability and reusability of code
• moving as many hardcoded numbers and flags into skill_db*.txt files (eg already mentioned skill damage ratios, certain skills that bypass X, etc)
• removing awful switch (skill_id) in favor of expanding internal skill database representation with function pointers to perform extra actions unique for given skill id (eg. overriding default skill damage ratio calculation, adding extra conditions or extra effects that would be too complex to represent in plain txt db files); in future allow overriding these functions by plugins for easy skill modifications

Opinions, suggestions?

I'm currently unable to commit due to my working copy being a mess because I'm working on the script update branch and am very unorganised about it xdd anyway I got this from hemagx, @reloadpcdb triggers pc_readdb which re-reads the files, thing is when skill tree data is modified and is then reload, online players data needs to be resync, in this case something similar to what skill_reload() does:

    /* lets update all players skill tree : so that if any skill modes were changed they're properly updated */
    iter = mapit_getallusers();
    for( sd = (TBL_PC*)mapit->first(iter); mapit->exists(iter); sd = (TBL_PC*)mapit->next(iter) )
        clif->skillinfoblock(sd);
    mapit->free(iter);

Fully Integrated.

Fully Integrated.