otland / forgottenserver

A free and open-source MMORPG server emulator written in C++
https://otland.net
GNU General Public License v2.0
1.57k stars 1.05k forks source link

using addEvent with item that has no uniqueId #3297

Open Azakelis opened 3 years ago

Azakelis commented 3 years ago

example:

function onUse(player, item, fromPosition, target, toPosition, isHotkey) local item = Item(item.uid) print(item:getName()) addEvent(test, 5000, item.uid) end

function test(uid) local item = Item(uid) if item == nil then print("item is nil") return end print(item:getName()) end

this will always output: itemname item is nil

althrough the item gets assigned uniqueid the scriptenv resets and with it the uniqueid assignemnt data is lost when event is executed

example quick solution that demonstrates how it should behave (tested, works as intended, code quality is meh): https://pastebin.com/qzEeYL63

MillhioreBT commented 3 years ago

Interesting proposal, although surely there is a cleaner and correct way to do it, for now this should have a feature tag

Note: it is worth mentioning that currently you can work with unique articles in addEvent, only you must know how to do it correctly, if you know what you want to do and you are clear about the result you can work well without the need for this that you propose

nekiro commented 3 years ago

Use issue template.

Zbizu commented 3 years ago

It's purposely done that way. If you need to track that item, you may add a custom attribute to it and check for that attribute whenever some interaction is triggered (like player moving an item for example).

Azakelis commented 3 years ago

I do get it what you all are saying, but I'd say the behavior is inconsistent comparing to other such cases (ex. Player) and your solutions seem to be workarounds for the existing problem. Sorry about the template, will stick to it next time.

Zbizu commented 3 years ago

I see, you probably want to construct Item(uid) for anything you want, but the items are different every read because they're kind of recreated when moved (https://github.com/otland/forgottenserver/blob/master/src/game.cpp#L1105) or there's another reason I'm not aware of.

nekiro commented 3 years ago

Yeah I wrote something similar to you some time ago. Your code will fail to obtain stackables that were modified (merged, split, even moved, stacks are entirely remade), guid will change and your code will fail to obtain your saved item by the guid. To make something like this work either remake stacking system (probably a lot of work) Had branch with this code, but I think I deleted it :(

gesior commented 3 years ago

Yeah I wrote something similar to you some time ago. Your code will fail to obtain stackables that were modified (merged, split, even moved, stacks are entirely remade), guid will change and your code will fail to obtain your saved item by the guid. To make something like this work either remake stacking system (probably a lot of work) Had branch with this code, but I think I deleted it :(

You can't use UID on stackables. Every implementation will be wrong. Add UID (UNIQUE ID!) to item with stack 100. Split it into 2 x 50 items, set that UID to both.. you got 2 items with same UID?! Now merge these 50 items stack into other 20 items stack. Do you add UID to that new 70 stack too? Now your UID works like virus.. and we get to final point: you got 2 stacks with different UIDs, merge them.. which UID do you keep?

You can add code like in this pastebin, but it should not work on stackable items (return error / nil / 0). Just make sure you don't crash server by manipulating on removed/virtual items.

It's purposely done that way. If you need to track that item, you may add a custom attribute to it and check for that attribute whenever some interaction is triggered (like player moving an item for example).

With 'do something on interaction' idea, you cannot use that item in addEvent and execute action like 'remove after 15 seconds'.

yamaken93 commented 3 years ago

I agree with everyone in this thread. My own implementation is a temp id, which obviously does not work for stackable items but can help a lot in certain situations.

Zbizu commented 3 years ago

With 'do something on interaction' idea, you cannot use that item in addEvent and execute action like 'remove after 15 seconds'.

Obviously not, but you can set that item decay to 15 seconds and add a new event to source code. Something that would trigger when item decay finishes.

MillhioreBT commented 3 years ago

With 'do something on interaction' idea, you cannot use that item in addEvent and execute action like 'remove after 15 seconds'.

Obviously not, but you can set that item decay to 15 seconds and add a new event to source code. Something that would trigger when item decay finishes.

It's not a bad idea

gesior commented 1 year ago

0.4 has a track of all items in Lua and you can access them by UID, if they were not removed or split [for stackable]. Somehow it detects all items that are removed from game.

Maybe in 1.x there should be special functionality to 'track Item' ex. Game.startTracking(item) and Game.getTrackedItem(item) that returns given Item or nil, if it was already delete. It would be very useful for many addEvent scripts. From implementation side, it looks like some std::map in game.h or luascripts.h with key Item* that is removed, when item is removed from game.