JukeBox 1.2

[Yek'-ta]

Yek'-ta submitted a new resource:

JukeBox - JukeBox

Read more about this resource...

 

[twisterniq]

1) You should use new const instead of define for strings.
2) SoundFile = SoundFile1 should be used copy instead.
3) get_pcvar_num(i_music) you can use bind_pcvar_num
4) You shouldn't use rg_find_ent_by_class with a custom classname.
5) You shouldn't check check if user is connected if you already check if user is alive.
6) if(enty != 0){ rh_emit_sound2(enty, 0, CHAN_VOICE, SoundFile, SOUNDVOLUME, SOUNDATT, 0, PITCH_NORM); } just unreadable
7) Why do you se HC_BREAK in not ReAPI hooks? Moreover, you shouldn't use HC_BREAK in PreThink. Why don't you use HC_CONTINUE?
8) format(g_MapFile, sizeof(g_MapFile), "maps/%s.juke.cfg", map) it's better to use formatex
9) Use charsmax instead of sizeof in natives like format, formatex...

 

[PurposeLess]

Also you should create a variable if you want to check if user is alive. It is overkill to check is_user_alive instead of g_alive[id]

Maybe switch

if(get_pcvar_num(i_music) == 1){
        SoundFile = SoundFile1
    }
    else if(get_pcvar_num(i_music) == 2){
        SoundFile = SoundFile2
    }
    else{
        SoundFile = SoundFile3
    }

 

[twisterniq]

Also you should create a variable if you want to check if user is alive. It is overkill to check is_user_alive instead of g_alive[id]

It's more reasonable to use is_user_alive to not break compatibility with other plugins which respawn players. An example could be a plugin which respawns players to make them ghosts (walking on the map being dead).

Maybe switch

Yes, it can be used. But I think it's better to use something like that to not hardcode:

new const g_szSounds[][] =
{
    "ambience/Opera.wav",
    "ambience/lv_jubilee.wav",
    "ambience/guit1.wav"
};

new g_szSoundFile[64];

copy(g_szSoundFile, charsmax(g_szSoundFile), g_szSounds[g_iCvarMusic - 1]);

 

[PurposeLess]

w0w,

It's more reasonable to use is_user_alive to not break compatibility with other plugins which respawn players. An example could be a plugin which respawns players to make them ghosts (walking on the map being dead).

I still think that variable is better. It is overkill to check is_user_alive.

It will be better, according to me.

Spawn(id)
{
    if(is_user_alive(id))
    {
        g_alive[id] = true;
    }
}

Killed(victim)
{
    g_alive[victim] = false;
}

PreThink(id)
{
    //if(!is_user_alive(id))
    if(!g_alive[id])
    {
        return;
    }
}

 

[twisterniq]

I still think that variable is better. It is overkill to check is_user_alive.

A little more optimized, nothing more. It can change the behavior of the plugin if a plugin such as I described is being used in the server. But if there is no plugin like that so no issues.