Alternate Equipment (Packet 1009: 45)

[pro4never]

Let me try to explain one last time where I feel you're making a mistake (not a huge one, just something that can be easily corrected)


#1: there's no reason to loop through ALL slots. We only care what is in the alternate gear position. If something is equipped in alternate headgear, we want it to be swapped with the current headgear. Simple as that.

#2: Certain stats cannot be accurately calculated until all gear slots have been updated. As such it's not a good plan to calculate stats inside the gear swap. Yes, there are ways you can write this all properly and leave it inside the loop... but it's easier and good practice to write re-usable code. The way you're describing is not reusable and is poor practice.


So yes... Here's some sample code (excuse the albetros references. It was the source I had handy)

#region Alternate Gear
                                    case ItemAction.SwapAltGears:
                                    case ItemAction.SwapStandardGears:
                                        for (ItemLocation slot = ItemLocation.ALTHead; slot < ItemLocation.ALTSteed; slot++)
                                        {
                                            var altItm = user.Equipment.GetItemBySlot(slot);
                                            if (altItm != null)
                                            {
                                                //Move current item into alternate gear slot (we don't care if it's null)
                                                user.Equipment.ChangeItemSlot(user.Equipment.GetItemBySlot(slot - 20), slot);
                                                //Move alternate item into current gear slot
                                                user.Equipment.ChangeItemSlot(altItm, slot - 20);
                                            }
                                        }
                                        //Now that we know ALL the items and their location. We can recalculate. 
                                        user.Recalculate();
                                        //Update your equipment to local players. 
                                        user.SendToScreen(Packet.SpawnPlayerPacket.Create(user), false);
                                        break;
                                    #endregion

[go for it]

Originally Posted by pro4never

Let me try to explain one last time where I feel you're making a mistake (not a huge one, just something that can be easily corrected)


#1: there's no reason to loop through ALL slots. We only care what is in the alternate gear position. If something is equipped in alternate headgear, we want it to be swapped with the current headgear. Simple as that.

#2: Certain stats cannot be accurately calculated until all gear slots have been updated. As such it's not a good plan to calculate stats inside the gear swap. Yes, there are ways you can write this all properly and leave it inside the loop... but it's easier and good practice to write re-usable code. The way you're describing is not reusable and is poor practice.


So yes... Here's some sample code (excuse the albetros references. It was the source I had handy)

#region Alternate Gear
                                    case ItemAction.SwapAltGears:
                                    case ItemAction.SwapStandardGears:
                                        for (ItemLocation slot = ItemLocation.ALTHead; slot < ItemLocation.ALTSteed; slot++)
                                        {
                                            var altItm = user.Equipment.GetItemBySlot(slot);
                                            if (altItm != null)
                                            {
                                                //Move current item into alternate gear slot (we don't care if it's null)
                                                user.Equipment.ChangeItemSlot(user.Equipment.GetItemBySlot(slot - 20), slot);
                                                //Move alternate item into current gear slot
                                                user.Equipment.ChangeItemSlot(altItm, slot - 20);
                                            }
                                        }
                                        //Now that we know ALL the items and their location. We can recalculate. 
                                        user.Recalculate();
                                        //Update your equipment to local players. 
                                        user.SendToScreen(Packet.SpawnPlayerPacket.Create(user), false);
                                        break;
                                    #endregion

and that's how alt. gears were released :P

[{ Angelius }]

Originally Posted by pro4never

#region Alternate Gear
                                    case ItemAction.SwapAltGears:
                                    case ItemAction.SwapStandardGears:
                                        for (ItemLocation slot = ItemLocation.ALTHead; slot < ItemLocation.ALTSteed; slot++)
                                        {
                                            var altItm = user.Equipment.GetItemBySlot(slot);
                                            [B]//this line alone loops through all gears to find an item with the slot X 
                                            //How many freaking cpu cycle does that waste lets say 9 alt slots 9*9 = [SIZE="4"]81[/SIZE] cycle? even worse when its standard slots[/B] 
                                            if (altItm != null)
                                            [B]//Why does it have to be null in the first place why wouldn't it just not be 
                                            //there at all i mean for real whats [B]efficient[/B] null stuff all over the server or keeping a 
                                            //sorteddictionary that contains only the items that are equipped and doesn't loop through null items for fun   [/B]
                                            {
                                                user.Equipment.ChangeItemSlot(user.Equipment.GetItemBySlot(slot - 20), slot);
                                                user.Equipment.ChangeItemSlot(altItm, slot - 20);
[B]                                                //Why wold you keep switching the items location instead of changing the item location on equip/un-equip and keeping 
                                                //them in a sorteddictionary with the location as a key like i did and 
                                                //how is it any better then a sorteddictionary [/B]
                                            }
[B]                                            [COLOR="Red"]//Every time the altItm = null its a wasted cpu cycle and we all know it
                                            //so if i am waring 1 single item which is the head gear your loop will test
                                            //every slot to find out that its = null except for the headgear
                                            //how is that [B]efficient[/B] and how does that help save cpu cycles? [/COLOR][/B]
                                        }
                                        user.Recalculate();
                                        user.SendToScreen(Packet.SpawnPlayerPacket.Create(user), false);
                                        break;
                                    #endregion

Originally Posted by pro4never

#1: there's no reason to loop through ALL slots. We only care what is in the alternate gear position. If something is equipped in alternate headgear, we want it to be swapped with the current headgear. Simple as that.

So you are saying its better to loop through all slots and waste precious cpu cycles that you are so concerned about the way you did it rather then having a dictionary that's always updated and contains no null items and keeps up with the number of items you are waring and doesn't waste cpu cycles when you loop through it?

Originally Posted by pro4never

#2: Certain stats cannot be accurately calculated until all gear slots have been updated. As such it's not a good plan to calculate stats inside the gear swap.

Yes it can be accurately calculated... And you should be able to calculate it whenever you need them that's A good advantage in cases 4/6 aka equip/un-equip in case 1009 and you don't have to wait until the user switch's to alt/standard gears to get a hold of the items stats to be able to accurately calculate them all together. And when/where the code user decides to do so... that's not my concern

After all it turns out your code is horribly inefficient. and it doesn't even worth looking at. And don't blame albetros references, Because you had the chance too clean/make all kind of changes to that code before you post it.

By the way my very first post was meant to help the thread owner nothing more and its not my fault if he/they misuse that few lines of coded that i posted..

[pro4never]

Originally Posted by { Angelius }

...

Ooh sweet lord...


GetItemBySlot is case statement, not a loop. This code is only run once (when swapping gear). GetItemBySlot is ANY way you wish to match up slot ID with the item structure. This can be a dictionary (perfectly valid way of doing things). There are no loops involved though.

Anyways, so done with this as you refuse to take any constructive criticism. This was a 2 minute sample for how the loop could be written without changing anything else in the source.

[.Xori^]

Originally Posted by pro4never

Ooh sweet lord...


GetItemBySlot is case statement, not a loop. This code is only run once (when swapping gear). GetItemBySlot is ANY way you wish to match up slot ID with the item structure. This can be a dictionary (perfectly valid way of doing things). There are no loops involved though.

Anyways, so done with this as you refuse to take any constructive criticism. This was a 2 minute sample for how the loop could be written without changing anything else in the source.

Hey there, I managed to fix the headgear not changing in the SwapGear function in albetros. I currently have the function 100% working using a command /swapgear but would like to get the button working. Little help? Heres what I got.

#region Swap Gear
                                    case ItemAction.SwapToAltGears:
                                    case ItemAction.SwapToMainGears:
                                        user.Equipment.SwapGear();
                                        break;
                                    #endregion

into Packethandler under case 1009.

What am I missing?

#edit, is the button controlled under DataAction.cs? if so what is the data type?

[EOS 60D]

its itemusage type = 45

[.Xori^]

Originally Posted by EOS 60D

its itemusage type = 45

Yea.. I already have that set. It's not working lol.

In fact, pressing the button doesn't even set off a breakpoint in packethandler.. What could the problem be?

-using Albetros source.

[EOS 60D]

Wait , what version you are currently working on?

[.Xori^]

Originally Posted by EOS 60D

Wait , what version you are currently working on?

Version 5518.