Jump to content

Unique Item ID Discussion


Ind

Recommended Posts


  • Group:  Members
  • Topic Count:  169
  • Topics Per Day:  0.04
  • Content Count:  1260
  • Reputation:   750
  • Joined:  11/19/11
  • Last Seen:  

This is something that has been requested for several times in the past, I'd like to know your opinions regarding this feature being built in.

Specs from my proposal:

  • Non-stackable items are given a unique ID stored as unsigned int 64 ( up to 18.446.744.073.709.551.615 )
    • Stackable items are not traceable in the current way they're stored; as when you get a red potion when you already have 10 the data from the new red potion is lost and the data that stores the 10 red potions gets it's amount += 1.

Post all your opinions, thank you for your time.

  • Upvote 5
Link to comment
Share on other sites


  • Group:  Members
  • Topic Count:  11
  • Topics Per Day:  0.00
  • Content Count:  427
  • Reputation:   123
  • Joined:  11/17/11
  • Last Seen:  

For what reason do we need uids?

To track down item duplication bugs? In that case, as far as I know, cards are stackable, still those are the most valuable items in the game. Is there any dupe report at all?

If no, and server owners want this feature to track corrupt GMs and stolen items from (hijacked) accounts, I think it is their responsibility to get good staff, and secure his/her control panel.

I think, it is not worth bothering implementing uids, because with the current desing it would be very time consuming to implement it properly, and with a secure CP and trusted GM staff it has no use.

Link to comment
Share on other sites


  • Group:  Members
  • Topic Count:  169
  • Topics Per Day:  0.04
  • Content Count:  1260
  • Reputation:   750
  • Joined:  11/19/11
  • Last Seen:  

fair points, just to clarify.

I think, it is not worth bothering implementing uids, because with the current desing it would be very time consuming to implement it properly, and with a secure CP and trusted GM staff it has no use.

the implementation is pretty straightforward actually, say you already have the field being loaded by char server into (struct item)->unique, in pc.c::pc_additem just the following addition would be necessary.

if( !item_data->unique && !itemdb_isstackable(data) )
     item_data->unique = the_global_unique_id_counter++;

Link to comment
Share on other sites


  • Group:  Members
  • Topic Count:  11
  • Topics Per Day:  0.00
  • Content Count:  427
  • Reputation:   123
  • Joined:  11/17/11
  • Last Seen:  

My problem is with stackable items, because if we add uids, users will definitely want them to work with cards.

And to proper tracking each card should have it's own uid in the stack.

Link to comment
Share on other sites


  • Group:  Members
  • Topic Count:  169
  • Topics Per Day:  0.04
  • Content Count:  1260
  • Reputation:   750
  • Joined:  11/19/11
  • Last Seen:  

My problem is with stackable items, because if we add uids, users will definitely want them to work with cards.

And to proper tracking each card should have it's own uid in the stack.

I understand that, which is why I previously said your previous reply contained fair points.

Link to comment
Share on other sites


  • Group:  Members
  • Topic Count:  33
  • Topics Per Day:  0.01
  • Content Count:  399
  • Reputation:   198
  • Joined:  11/09/11
  • Last Seen:  

From an administrative standpoint, this has always been something I've wanted; not for "item dupes" or "untrustworthy GMs", but for easier tracing of items that are misplaced or stolen between players/friends. It is a chore and a half to trace items, and unique item IDs would expedite this process. Ease of use; isn't that something wanted at the emulation level? This is something eAmod incorporates, as well as large scale servers I've worked for in the past, and is something I've always scratched my head over as to why eAthena never had it included in the emulator.

This notion gains a +1 from me.

  • Upvote 1
Link to comment
Share on other sites


  • Group:  Members
  • Topic Count:  17
  • Topics Per Day:  0.00
  • Content Count:  754
  • Reputation:   186
  • Joined:  05/22/12
  • Last Seen:  

here's a diff I've been keeping years ago. it is from sirius

And I'm to support jTynne's input on this. This would really help admins to keep track of their player's whereabouts.

serial-sirius_white.diff

  • Upvote 2
Link to comment
Share on other sites


  • Group:  Members
  • Topic Count:  72
  • Topics Per Day:  0.02
  • Content Count:  2997
  • Reputation:   1130
  • Joined:  05/27/12
  • Last Seen:  

So long as it doesn't mean a huge cost in performance or database size... why not? Nobody will mind an added convenience/safety measure, even if they don't have a use for it.

  • Upvote 1
Link to comment
Share on other sites


  • Group:  Developer
  • Topic Count:  30
  • Topics Per Day:  0.01
  • Content Count:  230
  • Reputation:   131
  • Joined:  11/21/11
  • Last Seen:  

I want this feature so badly. I had a server for 8 years and had a lot of problems tracking stolen/duped items... So I think it would be great we finally implement this :)

  • Upvote 2
Link to comment
Share on other sites


  • Group:  Members
  • Topic Count:  11
  • Topics Per Day:  0.00
  • Content Count:  427
  • Reputation:   123
  • Joined:  11/17/11
  • Last Seen:  

When I was running a server, our policy was: we never even tried to trace stolen stuff, because it is the player's mistake if it was stolen.

Every single time it turned out, the player gave out his or her password. However we developed our own CP, had our own packet encryption mechanism, and only stored salted password hashes in our database, so even GMs could not tell a player his or her password.

I admit that, the defaults of eA/rA is very insecure, so players usually can say that, they did not give away ( even if most of the time they did ), so indeed it is an useful feature.

If the implementation is not messy, I agree with adding it.

  • Upvote 2
Link to comment
Share on other sites


  • Group:  Members
  • Topic Count:  5
  • Topics Per Day:  0.00
  • Content Count:  341
  • Reputation:   43
  • Joined:  01/10/12
  • Last Seen:  

I want this feature so badly too! D:

No doubts, its an awesome feature, but imo it shouldn't be one of the priorities.

If any of the devs have extra time for it, then GO AHEAD! /no1

  • Upvote 2
Link to comment
Share on other sites

  • 2 weeks later...

  • Group:  Members
  • Topic Count:  146
  • Topics Per Day:  0.03
  • Content Count:  1195
  • Reputation:   467
  • Joined:  11/15/11
  • Last Seen:  

le bump

Link to comment
Share on other sites

  • 1 month later...

  • Group:  Members
  • Topic Count:  72
  • Topics Per Day:  0.02
  • Content Count:  2997
  • Reputation:   1130
  • Joined:  05/27/12
  • Last Seen:  

Is this being worked on, or why not?

  • Upvote 1
Link to comment
Share on other sites

  • 2 months later...

  • Group:  Members
  • Topic Count:  20
  • Topics Per Day:  0.00
  • Content Count:  213
  • Reputation:   109
  • Joined:  05/21/12
  • Last Seen:  

I agree with this suggestion. I love the idea, and it is very helpful (speaking as a server owner) to track.

We'll have to add it to the TO-DO list.

Marked as 'Started'. Assigned to Xantara. :) She is going to take care of this per our PM.

Thank you!

-Cookie

Link to comment
Share on other sites

  • 1 month later...

  • Group:  Members
  • Topic Count:  72
  • Topics Per Day:  0.02
  • Content Count:  2997
  • Reputation:   1130
  • Joined:  05/27/12
  • Last Seen:  

Xantara is finished with this project, but she is unable to run the final tests. I ask that the other Core Developers (and anyone else interested!) review and test the code before implementation. Thank you!

item_uid_v2.diff

  • Upvote 1
Link to comment
Share on other sites


  • Group:  Members
  • Topic Count:  6
  • Topics Per Day:  0.00
  • Content Count:  134
  • Reputation:   35
  • Joined:  02/27/12
  • Last Seen:  

I'm taking over it.

  • Upvote 2
Link to comment
Share on other sites

  • 2 weeks later...

  • Group:  Members
  • Topic Count:  6
  • Topics Per Day:  0.00
  • Content Count:  134
  • Reputation:   35
  • Joined:  02/27/12
  • Last Seen:  

Implemented Non Stackable item id at: r17080

I forgot to put on the commit, but credits go also to Xantara and Sirius White, which adapted to rA and created the base code, respectively.

I removed some unnecessary queries, renamed somethings, fixed some glitch and add a safer way to get the uid than the first, also put the option to enable disable and tested.

Link to comment
Share on other sites


  • Group:  Members
  • Topic Count:  9
  • Topics Per Day:  0.00
  • Content Count:  56
  • Reputation:   30
  • Joined:  02/22/12
  • Last Seen:  

PLEASE change the name of the unique id -.-

"nsiuid" is NOT very understandable to humans. Code should be written for anyone to understand just be reading it. Please code for humans not machines. I suggest changing it to "unique_id".

Both Xantara and Sirius_White had this base code written in an understandable way. MarkZD changed it from something understandable to something completely against basic standards of programming.

<+Kisuka> you're working on an open source project. code for people to understand without having to dig for info

<+Kisuka> if i hadn't updated my svn for 5 months i would have no idea wtf that column would be for in mysql until i dug thru the conf or history

<%MarkZD> a non dev doesn't need to know why a field is, only devs and database managers

<%MarkZD> ;D

<%MarkZD> any enterprise follow this rule

<+Kisuka> fuck u dude. I'm a developer and I didnt even know wtf it was until I went looking for ur info. What seperates a good programmer from a bad one is if your code is understandable to everyone just by fucking reading it. its basic common programming guidelines.

<%MarkZD> uhh, how bad u r

<+Kisuka> and im sorry but THIS ISNT AN ENTERPRISE CLOSED SRC PROJECT. Its open source. EVERYONE needs to able to understand it

<%MarkZD> ;D

<+Kisuka> not just the devs of rA

<%MarkZD> apply for dev and u can change or make a topic on rA suggestions

<%MarkZD> so we can check it

<%MarkZD> if it's in interest of all

Some core dev you guys have there -.- I'm disappointed. This guy has no idea wtf he's doing. Even if I was a new developer coming into rAthena, I would have to dig for information to understand what the fuck "nsiuid" means. Come on, who the fuck is this kid? How does he not know basic programming etiquette & standards? Even more so, why is he on the Core Dev Team if he doesn't know these standards for any open source project?

  • Upvote 1
Link to comment
Share on other sites


  • Group:  Members
  • Topic Count:  12
  • Topics Per Day:  0.00
  • Content Count:  318
  • Reputation:   37
  • Joined:  12/30/11
  • Last Seen:  

nsiuid = None Stackable Item Uniqe Item ID

i didnt know what it was too , after i read the changelog all was clear :)

so if ppl just update the Server whitout reading the changelogs

Link to comment
Share on other sites


  • Group:  Members
  • Topic Count:  9
  • Topics Per Day:  0.00
  • Content Count:  56
  • Reputation:   30
  • Joined:  02/22/12
  • Last Seen:  

nsiuid = None Stackable Item Uniqe Item ID

i didnt know what it was too , after i read the changelog all was clear :)

so if ppl just update the Server whitout reading the changelogs

What about if I was a new developer coming in a few months from now? Or someone interested in working on rAthena who has never played with it before but finds the project interesting? Code needs to be written for ALL to understand, not just a select group of people. It should NOT require you to go digging through commit history or conf files to understand what it means. This is basic programming standards & etiquette.

Not to mention, this is an open source project, this guideline should apply even MORE as you're coding for EVERYONE.

Link to comment
Share on other sites


  • Group:  Members
  • Topic Count:  6
  • Topics Per Day:  0.00
  • Content Count:  134
  • Reputation:   35
  • Joined:  02/27/12
  • Last Seen:  

-- OFF TOPIC START

Better the whole log:

<+Kisuka> uhhhh

<+Kisuka> MarkZD, nsiuid? that's totally understandable to any developer -.- code for a human not a machine. good practices please.

<+Kisuka> also is there a topic as to why you've implemented a unique id for non-stack items? I'd be interested in reading why.

<+Kisuka> nvm found it

<%MarkZD> nsiuid = non stackable item unique id, did u want me to type it all

<%MarkZD> ?

<%MarkZD> xD

<%MarkZD> it's wrote on the config

<%MarkZD> on the commit

<%MarkZD> description

<%MarkZD> on the topic

<%MarkZD> no need for more explanations about the name

<%MarkZD> e_e

<+Kisuka> doesnt matter. its still a crappy name for it. unique_id makes more sense and is understandable by just reading it

<%MarkZD> unique_id would be good if it was for all items

<Ai4rei> unique_id would be good either way

<+Kisuka> ^

<+Kisuka> you're working on an open source project. code for people to understand without having to dig for info

<+Kisuka> if i hadn't updated my svn for 5 months i would have no idea wtf that column would be for in mysql until i dug thru the conf or history

<%MarkZD> a non dev doesn't need to know why a field is, only devs and database managers

<%MarkZD> ;D

<%MarkZD> any enterprise follow this rule

<+Kisuka> fuck u dude. I'm a developer and I didnt even know wtf it was until I went looking for ur info. What seperates a good programmer from a bad one is if your code is understandable to everyone just by fucking reading it. its basic common programming guidelines.

<%MarkZD> uhh, how bad u r

<+Kisuka> and im sorry but THIS ISNT AN ENTERPRISE CLOSED SRC PROJECT. Its open source. EVERYONE needs to able to understand it

<%MarkZD> ;D

<+Kisuka> not just the devs of rA

<%MarkZD> apply for dev and u can change or make a topic on rA suggestions

<%MarkZD> so we can check it

<%MarkZD> if it's in interest of all

<+Kisuka> I've been a dev at eAthena and rAthena. I lost time to commit.

<%MarkZD> thanks

<+Kisuka> ive also been a dev for project janus and project tyr, ive been in this community way the fuck longer than you thank you VERY MUCH

<%MarkZD> cool

<+Kisuka> anyone decent programmer would agree that your naming in this case is completely horrible and not good practice. learn to accept critism of your code.

<%MarkZD> I'm, I'm not the one who need to be aggressive

<%MarkZD> I just said

<%MarkZD> use the suggestion forum

<%MarkZD> ;D

<%MarkZD> anyway I'll travel starting at 9, if u'r fast maybe I look at it before that

<%MarkZD> or another dev can see

<%MarkZD> I'll sleep now

<%MarkZD> have a nice day

As it's seem, I just explained the reason of name and I showed my point about why this name was chosen instead of unique_id.

He tells about my programming etiquette, but his human etiquette is worse.

This guy has no idea wtf he's doing.

Talked the guy, which says that is developer, which take hours to realize a mere descriptive name, which is explained for user, in configuration on src/config/core.h as mentioned on changelog.

-- OFF TOPIC END

uid is a commonly used name, the rest is just a complement, you should know it's a unique_id if you see uid, since you know every programming basic etiquette.

http://en.wikipedia.org/wiki/UID

http://docs.oracle.c...server/UID.html

I'm just here to help, as I said if the majority wants the name change so much I don't see really a problem, we can do it.

Code needs to be written for ALL to understand, not just a select group of people.

I agree and I'd like to know from other if it's so hard to know nsiuid is related to unique id.

Btw, I think nsiuid a good name, but it could be nsi_uid if it's easier for you to read, I think it's even better than unique_id for this case.

Link to comment
Share on other sites


  • Group:  Developer
  • Topic Count:  48
  • Topics Per Day:  0.01
  • Content Count:  1443
  • Reputation:   337
  • Joined:  10/17/12
  • Last Seen:  

The argument quotes are off topic,

Back on topic; I haven't been on rA long but I do agree that nsiuid is hard to read and understand, unique_id or nsi_uid would be much better, just imo

Btw I like this feature

  • Upvote 1
Link to comment
Share on other sites


  • Group:  Members
  • Topic Count:  9
  • Topics Per Day:  0.00
  • Content Count:  379
  • Reputation:   304
  • Joined:  11/10/11
  • Last Seen:  

I don't think we really need "nsi" in the name too, but I don't care.

In my own opinion this feature isn't important if it's just for tracing items. But it can help for script commands, when you want to modify/remove one specify item from a list that contain the same item multiples times.

Link to comment
Share on other sites


  • Group:  Members
  • Topic Count:  14
  • Topics Per Day:  0.00
  • Content Count:  351
  • Reputation:   52
  • Joined:  11/15/11
  • Last Seen:  

I have to agree with Kisuka here, unique_id is a lot easier to understand and really not that big of a deal to change into. I had to wait for ossi0110 to explain what it ment.

Link to comment
Share on other sites


  • Group:  Members
  • Topic Count:  1
  • Topics Per Day:  0.00
  • Content Count:  57
  • Reputation:   15
  • Joined:  12/25/11
  • Last Seen:  

I'd prefer a slightly different variable-naming as well. Unique_id or uid seems perfectly fine to me. This is not about updating without reading the change logs, whatsoever - it's more of a future reference issue. There's enough of confusing and/or redundant code all around the emulator already and code like this is not guaranteed to remain understandable in the future. Let's try to keep it in a cleaner way from now on at least.

On the whole this is not something to make such a fuss about, though - let's try to solve it in a mature way.

  • Upvote 1
Link to comment
Share on other sites

×
×
  • Create New...