Added player id to ItemRecv callback

[Daivuk]

Useful for me in Faxanadu, so I know the item doesn't come from me. Faxanadu is using ROM save states so I need to store the list of items received myself in the rom ram. So I can't use the "notify player" flag.

[N00byKing]

Can you elaborate on the use case? Don't really like this change as it is tbh...

[Daivuk]

It's one of the case where the API doesn't expose enough. In Faxanadu, I need to know if the item received is from my own world. By testing if the player is mine. (See my other PR where I added AP_GetPlayerId())

If you dont like how its done, don't worry. You can implement it in a different way that you like. I'll keep it like this in my fork for now.

[N00byKing]

I'd like to know what exactly it is being used for.
Why do you need to know which world the item came from?
Even then, there's an option to enable item recv messages to be queued up just like normal ones (AP_EnableQueueItemRecvMsgs), which exposes sender and receiver. Does that not work?

[Daivuk]

AP_EnableQueueItemRecvMsgs You don't pass the item id in there. You send it by name. Also if notify is false it won't send it.

I use it when I receive an item. I need to ignore my local items. Otherwise I will grab the item twice. It's common for NES/SNES games to have local items instead of remote in those cases.

[Daivuk]

Bottom line: I wishes we could have the full content of the packet through the API callbacks. Exposing more is always better because you can't predict what use-case the user will have.

The C# lib literally just convert the packet into a struct and pass it back to you. (+ do extra thing like caching datapackage obviously)

But again: it's really not a big deal if you want to keep it like that. I have my own changes in my fork. I will open those PRs anyway just incase its stuff you would like. I have 2 PRs more coming up later :)

[N00byKing]

We can agree do disagree on that; I want to expose as little as possible ^^'
The design of the public interface was meant to never give more information than needed. With the PlayerID, I already wanted to add it.
But for pretty much anything I am super hesitant to add it to the interface.

As for this change, you seem to want to emulate items_handling only for remote items. When I first wrote the library I never intended to support that, as I was feeling that that would not be the best approach for a game client anyway. However, I also never worked with games on an emulator interface, so it might very well be required there. I am still unsure of what to do in this case, will need to think about it, sorry.

Is it not possible to do full remote items in your scenario?

[Daivuk]

A flag to say only call this for remote item would work too if that's more in-line with your design philosophy :)
It is possible, but it will add a delay in-game on items that normally players would expect to react right away. Because they are local (And show their local sprites). Anyway, take your time. Its a small change that I can keep in my fork.

[Jarno458]

In somewhat simular fashion i added a boolean to my own verison of APCpp to state if the item comes from starting inventory or cheat commands, i use this to send item strait to a players inventory

                bool isFromServer = root[i]["items"][j]["location"].asInt64() < 0;
                (*getitemfunc)(item_id, notify, isFromServer);