Jump to content
Ind

Unique Item ID Discussion

Recommended Posts

-- 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.

I don't have an issue with uid; And yes, uid would have been a standard name, if you had used it. Instead you threw in nsi and didnt bother to even put a _ or anything. The following would have made MUCH more sense by just reading the column name:

  • unique_id
  • uid
  • serial

Here are some scenarios in which your "I left comments about it in the changelog" doesn't help at all:

  • It's 2 months from now. I'm a new core dev joining the team. My background in coding is impressive so I was brought on with not much experience in the community. I go to work on the src when I come across this variable. I know have to dig for the rest of the code or ask another dev what it's for as I have no idea what nsiuid means.
  • I'm a server owner who updates his svn very rarely (which most are), I update my svn, apply my diff (which most likely has a conflict now), compile and run my new build. I look at mysql a few days later, see the new column. I now have to dig to find what this column is for.
  • A year passes, the entire core dev team is replaced. Or the project dies and a new community is fomed. These new devs now have to learn this and any other bit of crypted variables you've used in the source.
  • I'm a programmer who is interested in MMORPG development. I find rA / eA by chance and want to use it as a base for my own development project. I now have to again, dig for information to find what the hell nsiuid means. As if this is the case, I've never seen the code or database once in my life.

To flat out say only Devs & DB Managers need to know what a variable name is, is COMPLETELY not in the spirit of an open source project. Athena is used by thousands of people for a wide range of projects, you are developing for EVERYONE, not just the core dev team, not just developers in general. You are a developer of an open source project. Your mission above EVERYTHING is to make code understandable by just reading it. Otherwise, you'll have developers looking at the code 3 months later going "wtf does this do again?".

Also I don't mean to be a dick but, how long have you been involved in the Athena community? The fact that you don't know my handle must mean you're pretty new. There's nothing wrong with that, however you don't seem to take opinions of your code very well. Yes, I was rude to you. I get angry when code is written in a messy fashion. If you're not passionate about programming and even more so not passionate about standards & clean code than you're doing something wrong as a programmer.

And for the record, it didn't take me "hours" to find out what it did. I updated my svn, saw a .sql file, looked at it, saw the field name than read the commit history and said "that's fucking stupid". Then messaged you on IRC about your horrible naming practice.

Share this post


Link to post
Share on other sites

@MarkZD

Is it planned to implement unique id's for stackable items? The most valuable items in most of the servers are the cards, so probably one wants to track them.

Even though the community seems to like this feature, I kinda dislike incomplete features. IMHO the gain of this feature (even if fully implemented, including support for stackable items) does not worth the complexity penalty of the code.

About the naming conventions, I think all of the proposed field name is ok, as long as it is clear from the documentation (of the corresponding config option) what it means.

@Kisuka

You should really learn to respect others. Your achievements do not make your superior and do not authorize you to write in this manner. Enumerating your contributions is not an excuse.

Share this post


Link to post
Share on other sites

@MarkZD

Is it planned to implement unique id's for stackable items? The most valuable items in most of the servers are the cards, so probably one wants to track them.

I'm planning about it, but it'd be a file where user would specify items, by name and some other thing as chance to get it to be unique and they'd be advised to only put rare items as it'd could take to severe performance issues if apple was defined as unique and people started to buy lot of them on NPC.

Lot of proccessing to create the uid's and insert items on database later.

Although, it's not yet sure we'll put it.

Share this post


Link to post
Share on other sites

Kisuka, with all due respect, I will not have you insulting any of our developers like that. MarkZD may be new, but he has shown himself to be as competent and devoted to the project as anyone else -- including you, for that matter. While I understand your objections, you should be well aware that yours was an inappropriate way of expressing them.

@xazax: A major issue with stackable items is the ability to create/delete 30,000 at one time (shops, commands, etc.). 30,000 queries would not bode well on server performance, but MarkZD's method may work so long as users have the sense not to create ridiculous amounts of the items.

@Keyworld: Do you have any suggestions regarding the implementation of related script commands?

  • Upvote 1

Share this post


Link to post
Share on other sites

Kisuka, with all due respect, I will not have you insulting any of our developers like that. MarkZD may be new, but he has shown himself to be as competent and devoted to the project as anyone else -- including you, for that matter. While I understand your objections, you should be well aware that yours was an inappropriate way of expressing them.

My sincerest apologies. I just don't like messy code. Furthermore, I don't enjoy someone saying only one group of people need to understand something in the source (I only got upset once he said this). It's a basic standard to program for others & humans. Honestly, even the name GUID would have been better and more understandable.

Yes, I'm a total asshole at times. However, I'm being an asshole for things which break standard practice. In my book, I'd rather have a guy yelling at me for writing messy code or implementing something wrong than being a outright jerk.

The code in athena is already bad enough as it is, lets not make it any worse please. We need to work toward cleaning it up and making it more understandable.

  • Upvote 2

Share this post


Link to post
Share on other sites
My sincerest apologies. I just don't like messy code. Furthermore, I don't enjoy someone saying only one group of people need to understand something in the source (I only got upset once he said this). It's a basic standard to program for others & humans. Honestly, even the name GUID would have been better and more understandable.

I got a little upset when I was just showing a point before saying that and trying to know about your point and you were agressive, so I said it. I know it's not every time true, I accept I was not fully right.

Yes, I'm a total asshole at times. However, I'm being an asshole for things which break standard practice. In my book, I'd rather have a guy yelling at me for writing messy code or implementing something wrong than being a outright jerk.

Thanks for showing your points anyway, it was just the way you started which was not good.

Ridicule is a thing, criticizes is another, and since start your were with the first option.

We(Kisuka & me) already talked on Irc and solved the above subject.

About the name, nsiuid will be changed later.

Renamed variables name from nsiuid to unique_id: r17086

  • Upvote 2

Share this post


Link to post
Share on other sites
@Keyworld: Do you have any suggestions regarding the implementation of related script commands?

About script commands:

Add:

hasuniqueitem(<item uid>) // check if you have it in your inventory
deluniqueitem(<item uid>) // delete an item

Modify:

getitem() / getitem2() / rentitem() // return item unique_id - getitem 1201, 1 {, [email protected]_uid_array };
getinventorylist() - add an uid array to the list.
successremovecards / failedremovecards / successrefitem / downrefitem - keep the item uid

And maybe add an optional "unique_item" argument to getitem2() to get RID of auto-magnifier NPC (delete item -> create item) ?

So now we can get RID of scripts like npc/jobs/2-1/blacksmith.txt:

                mes "[Geschupenschte]";
               mes "Oh, you should make sure that you are not carrying ^FF0000more than one "+getitemname([email protected][6])+"^000000, you should really only have an "+getitemname([email protected][6])+" that you bought from an NPC shop in your inventory.";
               next;
               if (select("Oh, could you give me a second?:Oh, I brought what you asked for.") == 1) {
                   mes "[Geschupenschte]";
                   mes "Hmmm, it would be";
                   mes "a good idea to put the";
                   mes "rest of your items";
                   mes "in Kafra Storage.";
                   close;
               }

Or to check integrity of an item you give to someone.

Well just some ideas.

  • Upvote 4

Share this post


Link to post
Share on other sites

For the unique item id for stackable items, wouldn't it be possible to create a file in /db/ that defines which stackable items to track so it can be limited to only the important ones such as MVP Cards.

Share this post


Link to post
Share on other sites

For the unique item id for stackable items, wouldn't it be possible to create a file in /db/ that defines which stackable items to track so it can be limited to only the important ones such as MVP Cards.

I'm planning about it, but it'd be a file where user would specify items, by name and some other thing as chance to get it to be unique and they'd be advised to only put rare items as it'd could take to severe performance issues if apple was defined as unique and people started to buy lot of them on NPC.

Lot of proccessing to create the uid's and insert items on database later.

Although, it's not yet sure we'll put it.

  • Upvote 1

Share this post


Link to post
Share on other sites

Perhaps it can be configured similar to the key "log_filter" in conf/log_athena.conf.

For example:

stackable_unique_id: 64   = use unique_id for cards (stackable item)

But always this configuration has to set carefully because of perfomance issues. :-)

Edited by HeikoS

Share this post


Link to post
Share on other sites

Perhaps it can be configured similar to the key "log_filter" in conf/log_athena.conf.

For example:

stackable_unique_id: 64   = use unique_id for cards (stackable item)

But always this configuration has to set carefully because of perfomance issues. :-)

Doing it that way may include more items than what certain servers may need. For example, A high rate server with 100% normal card drop rate does not need normal cards to tracked but rather MvP cards only.

Share this post


Link to post
Share on other sites

Right, i forgot this. An extra db.table is more customizable then.

Share this post


Link to post
Share on other sites

I don't know how you'll do this but I can already imagine cards/etc that have uid lose their stackability. xD

Share this post


Link to post
Share on other sites

I don't know how you'll do this but I can already imagine cards/etc that have uid lose their stackability. xD

Only in database, the struct will be the same for all items, being stackable with a pointer having its unique uid for each item, which will be used for non stackable as well to save a little ram.

I'll not give much more info about that now, I'm gonna organize my stuff to travel, see you after 16 days.

Share this post


Link to post
Share on other sites

The problem I see with stackable items is:

If a player has 2 cards with unique ids and trades one of them, how to decide which one to transfer?

For example when an account got hacked, often the stuff gets transfered over many chars and accounts and the admin wants to know which accounts were involved.

But when one of the chars already had one of the stackable items in the inventory before and now trades it to the next char, then if the wrong one gets transfered, the trace is lost and you have to falls back to the picklog again.

  • Upvote 1

Share this post


Link to post
Share on other sites

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?

Agreed. I was shocked to see those naming standards used. In fact, I also mentioned that it is signed versus unsigned on those integral data types. Hm... negative IDs... and not to mention the query execution time on the larger scale tables such as pick logs.

Share this post


Link to post
Share on other sites
The problem I see with stackable items is:

If a player has 2 cards with unique ids and trades one of them, how to decide which one to transfer?

then I suggest further for

1 for unique ID, and 2nd one for original unique ID

the original unique ID field is usually left 0

1. @item 501 2

create unique ID 1 to these 2 red potions

ID | ... | type | nameid | amount| ... | unique ID 1 | unique ID 2

1 | ... | A | 501 | 2 | ... | 1 | 0

2. @item 501 3

replace existing unique ID, since its stackable

ID | ... | type | nameid | amount| ... | unique ID 1 | unique ID 2

2 | ... | A | 501 | 3 | ... | 1 | 0

3. when drop on floor, trade, mail ... etc etc, that requires to unstack the items

create another unique ID (2) to those that has already stacked out

and unique ID 1 remains to that from the one getting unstack

-> drop 1 red potion on floor

ID | ... | type | nameid | amount| ... | unique ID 1 | unique ID 2

3 | ... | P | 501 | -1 | ... | 2 | 1

so when you want to trace item, you'll be able to trace that red pot with `unique ID 1` until 2

and you'll see `unique ID 2` field is not zero, then continue trace with `unique ID 1` field


http://www.eathena.ws/board/index.php?showtopic=185024

actually this has already been suggested before

but with that suggestion, it only meant to log non-stack-able items

Share this post


Link to post
Share on other sites

I know this one comes late, but I do not really get why we need this...

If you want to look up where a certain item has gone just walk through the itemlog. I did this very often for some stolen items, but I did not care if it was exactly the same item like the one that was stolen I just looked it up via account and item ID. I do not really care if the card I gave back to the original owner had been dropped by a monster he killed or something like that. I think it would be a nicer way solving such issues by putting more information about the items that are traded/dropped into the item log if there really is a problem, but I think every information is already in there, correct me if I am wrong. That does not consume as much performance and is not as complicated as the current solution...

The only important things for such cases in the logs would be:

  • Item dropped
  • Item dropped by looting type monster
  • Item traded
  • Item sold through shop
  • Item sold through auction
  • Item sent through mail
  • Item stored into guild storage

With this information you can find everything with the item log without needing some UID.

The problem is that if we implement UIDs to prevent item duplications is that it costs a lot of performance and that rAthena might miss out some bugs that will stay in the code until someone fixes them without knowing or by accident. I think that this is not a good solution either.

Edited by Lemongrass

Share this post


Link to post
Share on other sites

It would be really nice to have a "receiver" column in the picklog instead of 2 entries.

This way it would be easier to write a tool to track the path of the items and unuqie_id's wouldn't be neccessary for stackable items.

It would be really easy to do for instant owner changes of the item (like trades, vending or selling to a NPC), because we know both chars at the time of the creation of the picklog entry.

But the delayed owner changes (like drop&pickup or mail) are harder to solve...

One solution that came to my mind:

- Make the receiver column default to null.

- When someone drops a item we would have to create a new picklog_uid for it and add the picklog_uid to the picklog entry.

- When the item vanishes: UPDATE picklog SET picklog_uid = 0 WHERE picklog_uid = <picklog_uid>;

- When someone takes the item: UPDATE picklog SET receiver = <char_id>, picklog_uid = 0 WHERE picklog_uid = <picklog_uid>;

- On every Server start UPDATE picklog SET picklog_uid = 0 WHERE picklog_uid != 0 AND type IN(<all types where the item got lost on server shutdown (like drop)>);

picklog_uid == 0 means the log entry is finished, picklog_uid != 0 means the item doesn't have a new owner yet.

But I don't know how this solution would perform, with a huge picklog table the update querys could cripple the performance...

Share this post


Link to post
Share on other sites

I agree that this would be a lot better with a receiver getting logged. But to your solution I would say, that this should happen in rAthena in an in memory process.

Since the dropped items will not be stored as dropped if the char/map server does not save the user's session this should not be a problem. So I would say leave it for the 10 seconds or whatever the disappearance time of an item is in the memory, like you said with creating a UID for this and after it got picked up store it into the logs, because at this moment you can already tell which case happened and only run one query for it. That would also solve your server start problem which is far more complex than you stated, because you would also have to revert the step that was done to the user's inventory etc.

Share this post


Link to post
Share on other sites

You are right, I didn't consider the rollback part... and for the most part I would agree with your in memory idea.

But the inventory gets saved when a player quits the game, right?

In that case there would be a possibility, that logs are lost:

- Player A drops a item

- Player A quits the game

- The inventory gets saved, but the log is still delayed till we know the receiver

- The server crashes before the item vanishes or someone picks it up

- The log entry is lost

So we would need a combination of both our ideas... and it's getting more complex X_x

Share this post


Link to post
Share on other sites

You are partially right. The in memory implementation would have to be implemented session bound, which means, when the player drops a item it is stored with a UID in his session, if he quits the game, his session would get saved and all UIDs that have not been logged yet would be sent to the database.

As for the server crash, since a server crash also does not save the session, the inventory will be like he never dropped the item and therefore, no log would be needed.

And yeah basically it is a combination of the ideas. ;)

Share this post


Link to post
Share on other sites

I think logs should be write-only (only INSERT sql queries; never UPDATE or DELETE).

I don't know how you'll do this but I can already imagine cards/etc that have uid lose their stackability. xD

Only in database, the struct will be the same for all items, being stackable with a pointer having its unique uid for each item, which will be used for non stackable as well to save a little ram.

Yea that's what I was thinking too--NOT stacking them in sql, so each can be a separate row with its own unique_id (and the server "stacks" them before sending the inventory list to the client).

A /db/ file to list which stackable items will be tracked with UIDs

sounds like the most user-friendly way.

The problem I see with stackable items is:

If a player has 2 cards with unique ids and trades one of them, how to decide which one to transfer?

I suggest LIFO (pick the stackable item with the highest unique_id). I think this would fit the majority of trades/transfers involving multiple stackable items.

Share this post


Link to post
Share on other sites

successremovecards / failedremovecards / successrefitem / downrefitem - keep the item uid

really needed or it will not be coherent (in my server).

Share this post


Link to post
Share on other sites

Can this system as optional?

feature.unique_id: on/off

because this system is custom

no offense /heh

Share this post


Link to post
Share on other sites

×
×
  • Create New...

Important Information

By using this site, you agree to our Terms of Use and Privacy Policy.