Added player drops

[ErwinsExpertise]

Adding player drops. Item drops working as expected, however, meso drop essentially freezes player. Does the client expect a packet back confirming mesos change?

[ErwinsExpertise]

Relevant vana code: https://github.com/retep998/Vana/blob/19116c07dd0082b6c570bfd6d2ea604947773c10/src/channel_server/drop_handler.cpp#L198

[ErwinsExpertise]

Quantity drop is not 100% working but pickup/drop for mesos is 100% and single items/full stacks is working

[Hucaru]

Thanks for tackling this as it's not a simple feature to add with all the edge cases it has.

I tested this locally and found two issues:

1. The message for the amount of mesos received is not correct
2. When dropping an item and then picking it up followed by a relog, the item is gone from inventory, if using /drop command and picking up the items it spawns then they get saved, apart from the mesos, picking that up doesn't change the value in inventory.
3. With multiple characters in the same instance whenever and item is picked up all the characters receive the message.

EDIT: I made this a while ago to help me test things, you might find it useful https://github.com/Hucaru/maplestory-client-hook

[ErwinsExpertise]

@Hucaru thank you for the hook, that will definitely help.

I appreciate you taking time to review the PR.

As far as the mesos amount goes that's an issue I haven't been able to figure out. The packet is structured basically exactly like the Vana pickup notice yet the meso amount is not showing. I know you're a bit busy but if you have time could you review that specific packet? I'm really not sure what I'm doing wrong with that one.

This is the relevant code in Vana: https://github.com/retep998/Vana/blob/19116c07dd0082b6c570bfd6d2ea604947773c10/src/channel_server/drops_packet.cpp#L128

Everything else I'll get taken care of.

[Hucaru]

I think the dropping of items that have an amount greater than 1 such as potions should have the window pop up asking for the amount before the packet is sent, not really sure. It could be there is a property about the item (not the amount?) that tells the client when to do this. Might be worth looking in the item.go bytes function.

[ErwinsExpertise]

I'm not really sure how to get that to pop-up because it does not do it by itself in the client as it would with dropping mesos. The packet itself does not contain any unique identifiers either it simply has (opcode | pos | dropid). I'm assuming it is related to the item itself. I'll review that and see what I can find.

[ErwinsExpertise]

I believe they are handling the window via inventory operations here: https://github.com/retep998/Vana/blob/19116c07dd0082b6c570bfd6d2ea604947773c10/src/channel_server/inventory_handler.cpp#L113

Not too familiar with C++ so it's a little fuzzy but I'll work on it

[Hucaru]

I have had another play locally and it appears when dropping a potion and picking it up it can sometimes crash the client and the inventory for that item isn't handled properly i.e. I had a stack of 100 and 200 dropped 100, picked up 100 crashed loged in again and had a stack of 100 only in inventory.

EDIT: After more debugging it appears that items in the server and items in the client get out of sync. So the client thinks there are no potions in inventory but the server thinks there are. This means the new item flag is not set when picking the item up causing the crash as the client is told to update the amount in a slot that it thinks is empty. Sometimes dropping a stacked item and then picking it up followed by a relog makes it disappear.

[ErwinsExpertise]

I think that has to do with the saving inconsistencies. The server does not save as often as it should so I think it would be best to tackle that as a DB saving refactor.

Also I believe the dropping issue is relate to the item packets itself as you stated. I read an old ragezone post where it happened because the expiration time wasn't properly set. Will need to review more

[ErwinsExpertise]

I resolved the issue with the drop UI not showing by adding an expiration time. Not really sure why that matters, but it does apparently.

[ErwinsExpertise]

Pretty sure the stacking issue is related to the updateItem function. It looks like the slice is getting copied and we are modifying the copy slice. https://github.com/Hucaru/Valhalla/blob/354ba204ae26b73bf43df07b4859f3aac33ac575/channel/player.go#L685

[Hucaru]

Pretty sure the stacking issue is related to the updateItem function. It looks like the slice is getting copied and we are modifying the copy slice.

https://github.com/Hucaru/Valhalla/blob/354ba204ae26b73bf43df07b4859f3aac33ac575/channel/player.go#L685

That could very well be it. I don't have a specific test for it apart from moving stackable items around and dropping them then logging on and off. I think the general item management needs simplification as I suspect it's way more complicated than it needs to be.

[ErwinsExpertise]

Drops appear to be working as expected now. The copy slice was getting updated versus the players slice. In addition, when the item was added to the DB item.save function was not using a pointer so it was not updating the actual item, but rather a copy as well.

[ErwinsExpertise]

Basic ranged/magic skills attack skills are working. Need to figure out logic for things such as haste, etc.

Also I wish there was a way to easily convert Odin npc scripts to this type. There are an insane amount of npc's to script.

[Hucaru]

It might be worthwhile looking into changing the script system to just be the same as Odin. Have you finished the item related changes you wanted to make? I would like to keep the PRs scoped to a single area/topic/issue if possible.

[ErwinsExpertise]

Agreed, it would be a lot easier to mimic Odin versus making 300 different NPC scripts.

Yes all the changes are done and drops are working exactly as expected now from my testing.

[Hucaru]

There is a bug where if an item is spawned e.g. /item 2000003 100 then dropped and picked up, it is no longer in inventory when reloged (if partial amount dropped and picked up then works as expected). This applies to all item types. Would suggest there is a database saving issue when an item is picked up.

[ErwinsExpertise]

There is a bug where if an item is spawned e.g. /item 2000003 100 then dropped and picked up, it is no longer in inventory when reloged (if partial amount dropped and picked up then works as expected). This applies to all item types. Would suggest there is a database saving issue when an item is picked up.

Just tested and it looks like this was related to the drop item's DB ID. It kept the same db ID once dropped so changing it to have a db ID of 0 resolved the bug.

[Hucaru]

Happy for me to merge?

[ErwinsExpertise]

Yep, I'm working on skills which seems like it's going to be a lot. So this can merge