Code Newbies Helper Npc [Acis]

[disorder25]

Thank you Baggos, it work just fine with the Sp check.

Nice share and thank you for your reply and help.

[Tryskell]

- Each time you use giveItems, you generate useless arrays DaggerArmors, etc. You have to define static int arrays OUT of the method > http://stackoverflow.com/questions/10645914/are-local-variables-in-static-methods-also-static

- DaggerArmors.length == 0 is pointless as arrays you are referring are filled by yourself (you know the content) and never null/empty (supposed to be "final static" data).

- ItemInstance items = null; integrate it on the for loop, you earn nothing putting it here (except adding one line of code for nothing).

- NewbiesNpc class got no use, at all, as there is only one static method and (supposedly) a private static final int[] array. Content can be moved on L2NewbieNpcInstance or at least on L2NpcInstance if it has to be used on other classes.

 

--------------

+           ClassId classes = player.getClassId();
+           switch (classes)
+           {
+               case adventurer:
+               case sagittarius:
+               case duelist:
+               case titan:
+               case grandKhauatari:
+               case phoenixKnight:
+               case moonlightSentinel:
+               case fortuneSeeker:
+               case maestro:
+               case dreadnought:
+               case hellKnight:
+               case evaTemplar:
+               case swordMuse:
+               case windRider:
+               case shillienTemplar:
+               case spectralDancer:
+               case ghostHunter:
+               case ghostSentinel:
+               case soultaker:
+               case mysticMuse:
+               case archmage:
+               case arcanaLord:
+               case elementalMaster:
+               case cardinal:
+               case stormScreamer:
+               case spectralMaster:
+               case shillienSaint:
+               case dominator:
+               case doomcryer:
+                   NewbiesNpc.giveItems(0, player);
+                   break;
+           }

got strictly no use, as you just check few lines above :

+           if (currentClassId.level() < 3)
+           {

-------------------------

 

+           if (player.isMageClass() || player.getClassId() == ClassId.dominator || player.getClassId() == ClassId.doomcryer)
+           {
+               int mageSet[] = Config.NEWBIE_MAGE_BUFFS;
+               L2Skill buff;
+               for (int id : mageSet)
+               {
+                   buff = SkillTable.getInstance().getInfo(id, SkillTable.getInstance().getMaxLevel(id));
+                   buff.getEffects(this, player);
+                   player.setCurrentHp(player.getMaxHp());
+                   player.setCurrentCp(player.getMaxCp());
+                   player.setCurrentMp(player.getMaxMp());
+                   player.broadcastPacket(new MagicSkillUse(this, player, id, buff.getLevel(), 0, 0));
+               }
+           }
+           else
+           {
+               int fighterSet[] = Config.NEWBIE_FIGHTER_BUFFS;
+               L2Skill buff;
+               for (int id : fighterSet)
+               {
+                   buff = SkillTable.getInstance().getInfo(id, SkillTable.getInstance().getMaxLevel(id));
+                   buff.getEffects(this, player);
+                   player.setCurrentHp(player.getMaxHp());
+                   player.setCurrentCp(player.getMaxCp());
+                   player.setCurrentMp(player.getMaxMp());
+                   player.broadcastPacket(new MagicSkillUse(this, player, id, buff.getLevel(), 0, 0));
+               }
+           }

can be shortcuted to (on last aCis, isMageClass() is working as intented, but I leave your "hotfix")).

+               for (int id : (player.isMageClass() || player.getClassId() == ClassId.dominator || player.getClassId() == ClassId.doomcryer) ? Config.NEWBIE_MAGE_BUFFS : Config.NEWBIE_FIGHTER_BUFFS)
+               {
+                   L2Skill buff = SkillTable.getInstance().getInfo(id, SkillTable.getInstance().getMaxLevel(id));
+                   buff.getEffects(this, player);
+                   player.broadcastPacket(new MagicSkillUse(this, player, id, buff.getLevel(), 0, 0));
+               }

----

+ player.setCurrentHp(player.getMaxHp());
+ player.setCurrentCp(player.getMaxCp());
+ player.setCurrentMp(player.getMaxMp());

has to be setted up OUT of buff loop, otherwise you heal on every buff (which is pointless you only need one heal at the end).

Edited January 8, 2017 by Tryskell

['Baggos']

Thanks for your reply Tryskell..

Let me go home tomorrow, and I will give the second version..

I will try to use your reply about int arrays..

 

About heal yes.. I give the heal in every buff. I saw it yesterday..

I tried to add it without else but domi and doomcryer get fighter buffs..

I know on the latest aCis are mageclass.

 

About NewbiesNpc class, I put it here only because I won't it in L2NewbieNpcInstance.

 

In second version I have it on Startup class.

Edited January 8, 2017 by 'Baggos'

[Tryskell]

You should keep methods, variables and stuff in the "shortest range possible". Reading giveItems method, in fact you can even see there is no specific npc parameter. That method isn't related to npc, but player, as it holds 2 player parameters to work. Everything should be moved to L2PcInstance, as it's the shortest range. Rename giveItems to something more... "Catchy". It's way too much generic and can lead to huge mistakes.

 

A Startup class got no meaning, both as description (it doesn't startup anything or even load something, a static method being static) and concept - Startup.giveItems got no logic when you "say it loud", while player.giveNewbieItems() got a sense.

Edited January 8, 2017 by Tryskell

['Baggos']

I have everything in one class. Startup.

No need to throw anything to PcInstance I guess. Better to be everything in one place.

As you said contain only to player..

Easier for everyone to find it if they want to change the Items. (I won't put config for that.. Seems ugly because of many classes. Isn't only fighters/mages).

 

I hope the second version thanks to you, to be more cleaned and without more unnecessary checks/class.

 

Also, the second version doesn't contain npc.. Just a window onEnter..

Edited January 9, 2017 by 'Baggos'

[Sinister Smile]

Nice share mate! Can  be adapted for l2jfrozen? 

['Baggos']

Nice share mate! Can  be adapted for l2jfrozen?

 

I don't support frozen files..

[Benskis]

Nice share. Just 2 easy fixable problems: No Eva's Saint and should add xp check for lvl up as well.