Page 2 of 4

Re: doAttack Method...

Posted: Fri Aug 02, 2013 2:35 pm
by jurchiks
Come on man, the checks are there just to be safe, they do not harm anyone and null checks are the lightest checks that be, they are not resource-intensive or anything. What will you gain from removing that one check?

Re: doAttack Method...

Posted: Fri Aug 02, 2013 2:41 pm
by SpooKNoF
jurchiks wrote:Come on man, the checks are there just to be safe, they do not harm anyone and null checks are the lightest checks that be, they are not resource-intensive or anything. What will you gain from removing that one check?
5 checks for the same method npcs ai call again null check..
is ussules

Re: doAttack Method...

Posted: Fri Aug 02, 2013 2:47 pm
by UnAfraid
SpooKNoF wrote:
jurchiks wrote:Come on man, the checks are there just to be safe, they do not harm anyone and null checks are the lightest checks that be, they are not resource-intensive or anything. What will you gain from removing that one check?
5 checks for the same method npcs ai call again null check..
is ussules
I don't consider those null checks as useless at all.

Re: doAttack Method...

Posted: Fri Aug 02, 2013 2:48 pm
by SpooKNoF
UnAfraid wrote:
SpooKNoF wrote:
jurchiks wrote:Come on man, the checks are there just to be safe, they do not harm anyone and null checks are the lightest checks that be, they are not resource-intensive or anything. What will you gain from removing that one check?
5 checks for the same method npcs ai call again null check..
is ussules
I don't consider those null checks as useless at all.

not all of thems but inside doAttack yes it is..

is just a null check but how many checks we will get if we got 1000 players online? 5 same null checks for the same code.

Re: doAttack Method...

Posted: Fri Aug 02, 2013 2:52 pm
by jurchiks
What do you GAIN from removing it? It is not a lot of code, so not really a readability/simplicity improvement. It's totally not resource-intensive, so no performance gain either. It is there "just in case".
There are lots more places where things are double-checked, it's nothing terrible.

Re: doAttack Method...

Posted: Fri Aug 02, 2013 2:52 pm
by SpooKNoF
jurchiks wrote:What do you GAIN from removing it? It is not a lot of code, so not really a readability/simplicity improvement. It's totally not resource-intensive, so no performance gain either. It is there "just in case".
is just a null check but how many checks we will get if we got 1000 players online? 5 same null checks for the same code.

Re: doAttack Method...

Posted: Fri Aug 02, 2013 2:53 pm
by jurchiks
Who cares? It's just a null check. Find something more serious.

Re: doAttack Method...

Posted: Fri Aug 02, 2013 2:55 pm
by SpooKNoF
jurchiks wrote:Who cares? It's just a null check. Find something more serious.
is just ussules the null check on doAttack

if you are so good help l2j to start with small things and then you will find more serious :)

Re: doAttack Method...

Posted: Fri Aug 02, 2013 2:56 pm
by lord_rex
OH COME ON! Seriously.. Yesterday I had a nice day, was working siletly in peace, and than Intrepid came to annoy me with your topic(s) and told me that "There is a guys who is suffering with doAttack null checks for a week." and than I thought I gonna laugh on you a bit. But I have to ask that. Are you serious now or you're just joking?

If you're serious than: a null check (or even few more) may consumes 0.0000000000000000000000000000000000000000000000000001% of CPU per 100 players (just guessing by heart so don't start to be the clever guy if values not right!), why is it hurting your eyes?

and than, if you're sure about your statement, than GO and remove that null check and go in-game and check on every damn types of targets and you will realize that you will get THOUSANDS of NPEs. If not - than you were right! (huh, and?)

Re: doAttack Method...

Posted: Fri Aug 02, 2013 3:00 pm
by SpooKNoF
lord_rex wrote:OH COME ON! Seriously.. Yesterday I had a nice day, was working siletly in peace, and than Intrepid came to annoy me with your topic(s) and told me that "There is a guys who is suffering with doAttack null checks for a week." and than I thought I gonna laugh on you a bit. But I have to ask that. Are you serious now or you're just joking?

If you're serious than: a null check (or even few more) may consumes 0.0000000000000000000000000000000000000000000000000001% of CPU per 100 players (just guessing by heart so don't start to be the clever guy if values not right!), why is it hurting your eyes?

and than, if you're sure about your statement, than GO and remove that null check and go in-game and check on every damn types of targets and you will realize that you will get THOUSANDS of NPEs. If not - than you were right! (huh, and?)

WHY IS NOT USSULES EXPLAIN THAT HOW MANY NULL CHECKS WE USING FOR THE SAME METHOD? 4-5 HOW MANY TIMES 1 PLAYER use doAttack 99% for 1 player also npcs ai call another null check on hittimer we call again null check so IS USSULES!

Re: doAttack Method...

Posted: Fri Aug 02, 2013 3:01 pm
by lord_rex
SpooKNoF wrote:
lord_rex wrote:OH COME ON! Seriously.. Yesterday I had a nice day, was working siletly in peace, and than Intrepid came to annoy me with your topic(s) and told me that "There is a guys who is suffering with doAttack null checks for a week." and than I thought I gonna laugh on you a bit. But I have to ask that. Are you serious now or you're just joking?

If you're serious than: a null check (or even few more) may consumes 0.0000000000000000000000000000000000000000000000000001% of CPU per 100 players (just guessing by heart so don't start to be the clever guy if values not right!), why is it hurting your eyes?

and than, if you're sure about your statement, than GO and remove that null check and go in-game and check on every damn types of targets and you will realize that you will get THOUSANDS of NPEs. If not - than you were right! (huh, and?)

WHY IS NOT USSULES EXPLAIN THAT HOW MANY NULL CHECKS WE USING FOR THE SAME METHOD? 4-5 HOW MANY TIMES 1 PLAYER use doAttack 99% for 1 player also npcs ai call another null check on hittimer we call again null check so IS USSULES!
just one sentence: prove it

Re: doAttack Method...

Posted: Fri Aug 02, 2013 3:05 pm
by SpooKNoF
lord_rex wrote:
SpooKNoF wrote:
lord_rex wrote:OH COME ON! Seriously.. Yesterday I had a nice day, was working siletly in peace, and than Intrepid came to annoy me with your topic(s) and told me that "There is a guys who is suffering with doAttack null checks for a week." and than I thought I gonna laugh on you a bit. But I have to ask that. Are you serious now or you're just joking?

If you're serious than: a null check (or even few more) may consumes 0.0000000000000000000000000000000000000000000000000001% of CPU per 100 players (just guessing by heart so don't start to be the clever guy if values not right!), why is it hurting your eyes?

and than, if you're sure about your statement, than GO and remove that null check and go in-game and check on every damn types of targets and you will realize that you will get THOUSANDS of NPEs. If not - than you were right! (huh, and?)

WHY IS NOT USSULES EXPLAIN THAT HOW MANY NULL CHECKS WE USING FOR THE SAME METHOD? 4-5 HOW MANY TIMES 1 PLAYER use doAttack 99% for 1 player also npcs ai call another null check on hittimer we call again null check so IS USSULES!
just one sentence: prove it

lets start before we call doAttack we need to pass that method..

L2characterAi.java

/**
     * Manage the Attack Intention : Stop current Attack (if necessary), Start a new Attack and Launch Think
    @Override
    protected void onIntentionAttack(L2Character target)
    {
        if (target == null)
        {
            clientActionFailed();
            return;
        }


so we have 1 null check there.. again null check inside doAttack

doAttack call method hitTimer

public void onHitTimer(L2Character target, int damage, boolean crit, boolean miss, boolean soulshot, byte shld)
{
// If the attacker/target is dead or use fake death, notify the AI with EVT_CANCEL
// and send a Server->Client packet ActionFailed (if attacker is a L2PcInstance)
if ((target == null) || isAlikeDead() || (isNpc() && ((L2Npc) this).isEventMob()))
{
getAI().notifyEvent(CtrlEvent.EVT_CANCEL);
return;
}
again null check there you need more?

Re: doAttack Method...

Posted: Fri Aug 02, 2013 3:05 pm
by afk5min
The style is called 'Defensive Programming'. Has its benefits, especially when many people are working in parallel on the whole project – instead of each being assigned a subset of features to implement or maintain.

By the way, to determine if this null check is redundant, all you need to do is open up the call hierarchy (view references also works). The worst part is, you will have to do it on each new revision.
SpooKNoF wrote:if you are so good help l2j to start with small things and then you will find more serious :)
I beg to differ, that is not quite how it works. Surely, from a purist's PoV, every incorrectly working feature should be fixed. Even when it is a custom system message (identical) instead of client-stored message. The thing is, small things (such as sending initial UserInfo without clan data, to make sure retail server behavior is replicated) take time that can be spent better elsewhere, such as implementing new features, or reestablishing quick hacks/patchwork code.
Unfortunately, most people who try to bring attention to small details are not able to deal with more serious matters... And never really manage to learn.

P.S. Sure, you start 'finding more serious' stuff later on. But in the big picture, the whole l2j gameserver codebase is shit. Mostly because it became acceptable to rush feature implementations and then forget about them, reusing old code exclusively by copy-pasting. That wouldn't be so bad, but it only takes one person to make a mistake in his code, and that mistake will quickly propagate everywhere, but not by design, but as separate copies.

Re: doAttack Method...

Posted: Fri Aug 02, 2013 3:07 pm
by SpooKNoF
afk5min wrote:The style is called 'Defensive Programming'. Has its benefits, especially when many people are working in parallel on the whole project – instead of each being assigned a subset of features to implement or maintain.

By the way, to determine if this null check is redundant, all you need to do is open up the call hierarchy (view references also works). The worst part is, you will have to do it on each new revision.
SpooKNoF wrote:if you are so good help l2j to start with small things and then you will find more serious :)
I beg to differ, that is not quite how it works. Surely, from a purist's PoV, every incorrectly working feature should be fixed. Even when it is a custom system message (identical) instead of client-stored message. The thing is, small things (such as sending initial UserInfo without clan data, to make sure retail server behavior is replicated) take time that can be spent better elsewhere, such as implementing new features, or reestablishing quick hacks/patchwork code.
Unfortunately, most people who try to bring attention to small details are not able to deal with more serious matters... And never really manage to learn.

P.S. Sure, you start 'finding more serious' stuff later on. But in the big picture, the whole l2j gameserver codebase is shit. Mostly because it became acceptable to rush feature implementations and then forget about them, reusing old code exclusively by copy-pasting. That wouldn't be so bad, but it only takes one person to make a mistake in his code, and that mistake will quickly propagate everywhere, but not by design, but as separate copies.

why you bring more people to proove what? IS NOT USSULES NULL CHECK?

Re: doAttack Method...

Posted: Fri Aug 02, 2013 3:09 pm
by afk5min
SpooKNoF wrote:why you bring more people to proove what? IS NOT USSULES NULL CHECK?
afk5min wrote:The style is called 'Defensive Programming'. Has its benefits, especially when many people are working in parallel on the whole project – instead of each being assigned a subset of features to implement or maintain.
afk5min wrote:By the way, to determine if this null check is redundant, all you need to do is open up the call hierarchy (view references also works). The worst part is, you will have to do it on each new revision.