ItemSortHandler fix

[StripedCow]

I am bored, so i'm releasing something useless(well not really useless because i can crash servers with this), but yeah.

Enjoy.

/*
This file is part of the OdinMS Maple Story Server
Copyright (C) 2008 Patrick Huy <patrick.huy@frz.cc>
Matthias Butz <matze@odinms.de>
Jan Christian Meyer <vimes@odinms.de>

This program is free software: you can redistribute it and/or modify
it under the terms of the GNU Affero General Public License as
published by the Free Software Foundation version 3 as published by
the Free Software Foundation. You may not use, modify or distribute
this program under any other version of the GNU Affero General Public
License.

This program is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
GNU Affero General Public License for more details.

You should have received a copy of the GNU Affero General Public License
along with this program.  If not, see <http://www.gnu.org/licenses/>.
 */
package communication.channel.handlers;

import communication.MapleClient;
import communication.AbstractMaplePacketHandler;
import character.inventory.MapleInventoryType;
import character.inventory.MapleInventory;
import server.MapleInventoryManipulator;
import packet.MaplePacketCreator;
import packet.input.SeekableLittleEndianAccessor;

public class ItemSortHandler extends AbstractMaplePacketHandler {

    public void handlePacket(SeekableLittleEndianAccessor slea, MapleClient c) {
        slea.readInt(); // timestamp
        byte mode = slea.readByte();
        boolean sorted = false;
        MapleInventoryType pInvType = MapleInventoryType.getByType(mode);
        MapleInventory pInv = c.getPlayer().getInventory(pInvType);
        if (pInv == null || pInv.isFull()) {
            return;
        }
        while (!sorted) {
            byte freeSlot = pInv.getNextFreeSlot();
            if (freeSlot != -1) {
                byte itemSlot = -1;
                for (int i = freeSlot + 1; i <= pInv.getSlotLimit(); i++) {
                    if (pInv.getItem((byte) i) != null) {
                        itemSlot = (byte) i;
                        break;
                    }
                }
                if (itemSlot <= 100 && itemSlot > 0) {
                    MapleInventoryManipulator.move(c, pInvType, itemSlot, freeSlot);
                } else {
                    sorted = true;
                }
            }
        }
        c.getSession().write(MaplePacketCreator.finishedSort(mode));
        c.getSession().write(MaplePacketCreator.enableActions());
    }
}

Bye.

 

[Darksta]

ill test this out when i get home, but based on passed reputation it should work. well.

 

[Sharky]

ill test this out when i get home, but based on passed reputation it should work. well.

Haha you high?

Great release amigo.

 

[Soulfist]

Haha you high?

Great release amigo.

Although what he said made no sense, he probably means well. And the use of amigo brings the word "racist" to mind, I guess its conditioning in being around so many ignorant kids.

@ricky
This is very simple, but useful, but to be honest its common sense. But then again most people lack even that so good job nonetheless.

 

[kevintjuh93]

Now how about RangedAttackHandler.java?
final Item item = player.getInventory(MapleInventoryType.USE).getItem((byte) attack.slot);

Going through the whole inventory is just lame, when the packet send the slot with it.

        if (ranged) {
            ret.slot = lea.readShort();//starSlot
            ret.cslot = lea.readShort();//cashSlot

            ret.rangedirection = lea.readByte(); // aoe?
        }

 

[orikiller]

its for xeonms? i dont understand what this fix..

 

[Chance]

I don't know how this is a fix but I guess unlimited slots are cool.

 

[kevintjuh93]

Use; mode >= 1 && mode <= 5

So.. if my inventory is full, this means I may not get to ItemIdSort?

There still an infinite loop too.

 

[SuperLol]

Use; mode >= 1 && mode <= 5

So.. if my inventory is full, this means I may not get to ItemIdSort?

There still an infinite loop too.

What? This isn't item id sort. Why would you want to merge slots when your inventory is full?

Infinite loop should only happen if sorted is never true, which can only happen if:
-freeSlot != -1, which isn't possible because the inventory is not full. A previous check passes it.
-i <= pInv.getSlotLimit() is always true, which is impossible because i is incremented every time by 1. Also getSlotLimit has a max of 96.
- item slot is <= 100 and itemslot > 0 is always true, which it cannot because after i <= pInv.getSlotLimit() is satisfied, then the itemslot will always be < 0. It should still say getSlotLimit() instead of 100 though, but that won't change anything.

So basically there is no infinite loop from what I can see.

 

[Soulfist]

@ Kevin

Not how I would do it, but you're correct.

@ Ricky

You know, I told you that you were wrong, so to show you I looked at the standard ItemSortHandler in MapleBlade (yes) and redid it, working better than yours, and fully fixing the problem at that.

/*
This file is part of the OdinMS Maple Story Server
Copyright (C) 2008 Patrick Huy <patrick.huy@frz.cc>
Matthias Butz <matze@odinms.de>
Jan Christian Meyer <vimes@odinms.de>

This program is free software: you can redistribute it and/or modify
it under the terms of the GNU Affero General Public License as
published by the Free Software Foundation version 3 as published by
the Free Software Foundation. You may not use, modify or distribute
this program under any other version of the GNU Affero General Public
License.

This program is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
GNU Affero General Public License for more details.

You should have received a copy of the GNU Affero General Public License
along with this program.  If not, see <http://www.gnu.org/licenses/>.
 */
package net.channel.handler;

import client.MapleClient;
import client.MapleInventoryType;
import client.MapleInventory;
import server.MapleInventoryManipulator;
import net.AbstractMaplePacketHandler;
import tools.MaplePacketCreator;
import tools.data.input.SeekableLittleEndianAccessor;

public class ItemSortHandler extends AbstractMaplePacketHandler {

    @Override
    public final void handlePacket(SeekableLittleEndianAccessor slea, MapleClient c) {
        //c.getPlayer().resetAfkTime();
        slea.readInt(); // timestamp
        boolean sorted = false;
        byte mode = slea.readByte(), itemSlot = -1, freeSlot = -1;
        MapleInventoryType type = MapleInventoryType.getByType(mode);
        MapleInventory pInv = c.getPlayer().getInventory(type);
        
        while (!sorted && pInv != null) {
            if ((freeSlot = pInv.getNextFreeSlot()) == -1) break;
            for (int i = freeSlot + 1; i <= pInv.getSlotLimit(); i++) {
                if (pInv.getItem((byte) i) != null) {
                    itemSlot = (byte) i;
                    break;
                }
            }
            if (itemSlot <= pInv.getSlotLimit() && itemSlot > 0)
                MapleInventoryManipulator.move(c, type, itemSlot, freeSlot);
            else
                sorted = true;
        }
        c.getSession().write(MaplePacketCreator.finishedSort(mode));
        c.getSession().write(MaplePacketCreator.enableActions());
    }
}

What? This isn't item id sort. Why would you want to merge slots when your inventory is full?

Infinite loop should only happen if sorted is never true, which can only happen if:
-freeSlot != -1, which isn't possible because the inventory is not full. A previous check passes it.
-i <= pInv.getSlotLimit() is always true, which is impossible because i is incremented every time by 1. Also getSlotLimit has a max of 96.
- item slot is <= 100 and itemslot > 0 is always true, which it cannot because after i <= pInv.getSlotLimit() is satisfied, then the itemslot will always be < 0. It should still say getSlotLimit() instead of 100 though, but that won't change anything.

So basically there is no infinite loop from what I can see.

It can still have an infinite loop. As when getSlotLimit() returns, if it's full it will return -1, hence the original check put there by the person who made the original handler. However, the problem is that, that doesn't end the while() loop, only the for loop, making it redundant, sort will never be finished because while will skip over everything in it's body and keep repeating.
You can either set sort to true upon finding a -1, or you can break, which I did (break), as I think it would work a little better than the other method.

Edit: Yes, I realize that the previous, abrupt if statement is placed there already to return the entire program if the inventory is full, but it's not the best way to handle it, and it's redundant as it's already handled by the method used to check slotlimit, as I said.

 

[SuperLol]

It will break the for loop and go to the if statement. The first condition will be false because itemSlot will be -1, so then it will say it is done. I still don't see how the code is broken as it works. I used a similar one for 2 years and it hasn't caused any problems. Someone should try modifying the code to run while (pInv.getNextFreeSlot() != -1) or something.

Also no need to check if the inventory is null every time.

 

[kevintjuh93]

What? This isn't item id sort. Why would you want to merge slots when your inventory is full?

Infinite loop should only happen if sorted is never true, which can only happen if:
-freeSlot != -1, which isn't possible because the inventory is not full. A previous check passes it.
-i <= pInv.getSlotLimit() is always true, which is impossible because i is incremented every time by 1. Also getSlotLimit has a max of 96.
- item slot is <= 100 and itemslot > 0 is always true, which it cannot because after i <= pInv.getSlotLimit() is satisfied, then the itemslot will always be < 0. It should still say getSlotLimit() instead of 100 though, but that won't change anything.

So basically there is no infinite loop from what I can see.

Yeah saw that later. If the finish sort packet isn't send, you will never be able to get through the ItemIdSortHandler.