Support itemids up to ~2 billion #5141

vstumpf · 2020-06-24T05:09:33Z

Addressed Issue(s):

If anyone knows the issue number, pls comment

Server Mode:

Both

Description of Pull Request:

I've been working on this for a year and a half pls no hate
item ids are now uint32

ty @aleos89 for helping

lgtm-com · 2020-06-24T05:41:46Z

This pull request introduces 2 alerts when merging d98d205b5e946fa825727019e9e8755de391b8f6 into d35e289 - view on LGTM.com

new alerts:

2 for Comparison result is always the same

sader1992 · 2020-06-24T16:33:44Z

I think with this the max storage etc is now 682 ? (struct size with 682 65509)

Badarosk0 · 2020-06-25T02:23:09Z

With this PR will it be possible to create items with larger IDs? for example:

100052,Enchant_Stone_Box19,Costume Enchantment Stone Box 19,2,10,,10,,,,0,0xFFFFFFFF,63,2,,,1,,,{ getgroupitem(IG_Enchant_Stone_Box19); },{},{}

I applied and the items cannot be created.

Another problem I noticed is that the HP and SP in the char is now 1/1.

ecdarreola · 2020-06-25T02:55:16Z

n the char is now 1/1.

Unable to reproduce for the high ITEM ID not tradeable. (See below)

As for the HP, its also working fine.

vstumpf · 2020-06-25T02:56:34Z

Sorry, I should have said that this is only for clients that support it (ones that support 32bit item ids)
That means these clients:
PACKETVER_MAIN_NUM >= 20181121 || PACKETVER_RE_NUM >= 20180704 || PACKETVER_ZERO_NUM >= 20181114

Badarosk0 · 2020-06-25T02:57:47Z

Weird. I just downloaded the clean rAthena and applied it. I also applied the 2 sql upgrade files. I'm using 2020-04-01.

src/map/script.cpp

lgtm-com · 2020-06-25T03:23:46Z

This pull request introduces 2 alerts when merging 1f0fc488e45a0e63c999fcfac20f91f911205a10 into c14f05b - view on LGTM.com

new alerts:

2 for Comparison result is always the same

Badarosk0 · 2020-06-25T03:31:15Z

There are 2 warnings after compilation:

Warning C4244 '=': conversion from 'unsigned long' to 'short', possible loss of data char-server trunk\src\char\int_auction.cpp line 250

Warning C4146 unary minus operator applied to unsigned type, result still unsigned map-server trunk\src\map\script.cpp line 7901

qwerty7vp · 2020-06-25T03:44:47Z

Unable to reproduce for the high ITEM ID not tradeable. (See below)
As for the HP, its also working fine.

confirmed that I can trade
and HP SP worked fine

Badarosk0 · 2020-06-25T04:34:36Z

It worked now, for some reason the rA is like PRERE as a default now and I didn't notice.
It's working now.

src/config/renewal.hpp

ecdarreola

Also, under script.cpp (makeitem)

The line

causes a warning below.:

ecdarreola · 2020-06-25T08:37:01Z

src/char/int_auction.cpp

 		}

 		for (i = 0; i < MAX_ITEM_RDM_OPT; i++) {
 			Sql_GetData(sql_handle, 15 + MAX_SLOTS + i*3, &data, NULL);
-			item->option[i].id = atoi(data);
+			item->option[i].id = strtoul(data, NULL, 10);


This causes a warning upon compiling. (possible loss of data)

Interesting, this shouldn't have been changed, since random option IDs haven't changed.
I'll fix

Badarosk0 · 2020-06-25T16:19:07Z

Also, under script.cpp (makeitem)

The line

causes a warning below.:

i can confirm this.

lgtm-com · 2020-06-25T17:15:15Z

This pull request fixes 1 alert when merging 0c93af0f57f38ec4f1225a3a451e5a5e7deb5d11 into 9e79d59 - view on LGTM.com

fixed alerts:

1 for Comparison result is always the same

ecdarreola · 2020-06-25T17:34:21Z

src/map/script.cpp

@@ -7942,7 +7940,7 @@ BUILDIN_FUNC(makeitem2) {
 			nameid = UNKNOWN_ITEM_ID;
 	}
 	else
-		nameid = script_getnum(st, 2);
+		nameid = script_getnum64(st, 2);


possible loss of data from int64 to uint32.

🤦 dont know why i changed this

aleos89

Another note: all the script commands will have to be adjusted to something like nameid = static_cast<uint32>(script_getnum64(st, X)); since script_getnum is int.

aleos89 · 2020-06-25T19:31:42Z

src/map/cashshop.hpp

@@ -6,6 +6,7 @@

 #include "../common/cbasetypes.hpp" // uint16, uint32
 #include "../common/timer.hpp" // ShowWarning, ShowStatus
+#include "../common/mmo.hpp"


This isn't needed anymore.

src/map/log.hpp

* Reverted a print specifier.

* Adjusted main SQL files from smallint(5) to int(10). * Added upgrade SQL scripts.

vstumpf · 2020-07-19T22:29:32Z

Sorry about that @Badarosk0, I must have merged something incorrectly. It should be fixed now!

lgtm-com · 2020-07-19T23:09:06Z

This pull request fixes 1 alert when merging f8227e3 into 9000948 - view on LGTM.com

fixed alerts:

1 for Comparison result is always the same

Forte22 · 2020-07-20T19:56:18Z

Do you have a rough idea how much longer this PR could take to get pushed to master?

Badarosk0 · 2020-07-20T20:02:32Z

Do you have a rough idea how much longer this PR could take to get pushed to master?

I believe that the more people who test and submit problems here, the faster it will be.

hsiaonaiwen · 2020-08-04T11:33:34Z

Is using a taming item capturing pet working fine?

Badarosk0 · 2020-08-04T14:55:08Z

Is using a taming item capturing pet working fine?

work fine for me

lgtm-com · 2020-08-07T14:05:03Z

This pull request fixes 1 alert when merging 386f8ef into 53e7f59 - view on LGTM.com

fixed alerts:

1 for Comparison result is always the same

lgtm-com · 2020-08-07T14:30:47Z

This pull request fixes 1 alert when merging 1646094 into 53e7f59 - view on LGTM.com

fixed alerts:

1 for Comparison result is always the same

aleos89

Looking great! Just a couple things I spotted after a run-through.

src/map/script.cpp

src/map/skill.cpp

src/map/script.cpp

lgtm-com · 2020-08-07T20:08:00Z

This pull request fixes 1 alert when merging a404cda into c86a5a4 - view on LGTM.com

fixed alerts:

1 for Comparison result is always the same

Should give better performance during migration Added missing newline at end of file

Should match the naming schema

lgtm-com · 2020-08-08T10:02:35Z

This pull request fixes 1 alert when merging 3078847 into fd148a6 - view on LGTM.com

fixed alerts:

1 for Comparison result is always the same

This reverts commit 3776bfb.

vstumpf changed the title ~~Support itemids up to ~2 million~~ Support itemids up to ~2 billion Jun 24, 2020

ecdarreola reviewed Jun 25, 2020

View changes

src/map/script.cpp Show resolved Hide resolved

Badarosk0 reviewed Jun 25, 2020

View changes

src/config/renewal.hpp Outdated Show resolved Hide resolved

ecdarreola reviewed Jun 25, 2020

View changes

aleos89 requested changes Jun 25, 2020

View changes

vstumpf and others added 12 commits Jun 26, 2020

20% done

8670465

clif is mostly done

6e189ea

70% done

412b452

clean compiles

07db7e5

change type name, get rid of some ifdefs

d3dd627

only use typedef for packets

6bc22a0

Converted msg_table strings to use %u versus %hu

4aec019

Small revert

2d3bb42

* Reverted a print specifier.

Added SQL file changes

db6d274

* Adjusted main SQL files from smallint(5) to int(10). * Added upgrade SQL scripts.

updated

646c2c8

hu? u

352dfd8

Update packetvers

5a1121c

Lemongrass3110 added 2 commits Jul 28, 2020

Changed start items and positions

7810f52

Cleanup

780d900

aleos89 mentioned this pull request Jul 29, 2020

Several atcommands uses atoi on itemdb_exists which accepts unsigned short #4035

Closed

aleos89 linked an issue Jul 29, 2020 that may be closed by this pull request

Several atcommands uses atoi on itemdb_exists which accepts unsigned short #4035

Closed

Massive cleanup

386f8ef

Follow up to 386f8ef

1646094

aleos89 reviewed Aug 7, 2020

View changes

src/map/script.cpp Outdated Show resolved Hide resolved

src/map/skill.cpp Outdated Show resolved Hide resolved

Apply suggestions from code review

858deed

aleos89 approved these changes Aug 7, 2020

View changes

src/map/script.cpp Outdated Show resolved Hide resolved

Apply suggestions from code review

a404cda

Lemongrass3110 added 4 commits Aug 8, 2020

Combined alter statements

3dcdabf

Should give better performance during migration Added missing newline at end of file

Renamed upgrade files

eb9104b

Should match the naming schema

Adapted missing premium storage script

da46049

Added sales table migration

3078847

Lemongrass3110 added component:core mode:prerenewal mode:renewal type:bug labels Aug 8, 2020

Lemongrass3110 merged commit 3776bfb into rathena:master Aug 8, 2020
4 checks passed

SeravySensei added a commit to SeravySensei/rathena that referenced this issue Aug 13, 2020

Revert "Support itemids up to ~2 billion (rathena#5141)"

fda22bd

This reverts commit 3776bfb.

rathena / rathena Public

Support itemids up to ~2 billion #5141

Support itemids up to ~2 billion #5141

vstumpf commented Jun 24, 2020

lgtm-com bot commented Jun 24, 2020

sader1992 commented Jun 24, 2020

Badarosk0 commented Jun 25, 2020 •

edited

ecdarreola commented Jun 25, 2020

vstumpf commented Jun 25, 2020

Badarosk0 commented Jun 25, 2020

lgtm-com bot commented Jun 25, 2020

Badarosk0 commented Jun 25, 2020

qwerty7vp commented Jun 25, 2020

Badarosk0 commented Jun 25, 2020

ecdarreola left a comment

ecdarreola Jun 25, 2020

vstumpf Jun 25, 2020

Badarosk0 commented Jun 25, 2020

lgtm-com bot commented Jun 25, 2020

ecdarreola Jun 25, 2020

vstumpf Jun 26, 2020

aleos89 left a comment

aleos89 Jun 25, 2020

vstumpf commented Jul 19, 2020

lgtm-com bot commented Jul 19, 2020

Forte22 commented Jul 20, 2020

Badarosk0 commented Jul 20, 2020

hsiaonaiwen commented Aug 4, 2020

Badarosk0 commented Aug 4, 2020

lgtm-com bot commented Aug 7, 2020

lgtm-com bot commented Aug 7, 2020

aleos89 left a comment

lgtm-com bot commented Aug 7, 2020

lgtm-com bot commented Aug 8, 2020

rathena / rathena Public

Support itemids up to ~2 billion #5141

Support itemids up to ~2 billion #5141

Conversation

vstumpf commented Jun 24, 2020

lgtm-com bot commented Jun 24, 2020

sader1992 commented Jun 24, 2020

Badarosk0 commented Jun 25, 2020 • edited

ecdarreola commented Jun 25, 2020

vstumpf commented Jun 25, 2020

Badarosk0 commented Jun 25, 2020

lgtm-com bot commented Jun 25, 2020

Badarosk0 commented Jun 25, 2020

qwerty7vp commented Jun 25, 2020

Badarosk0 commented Jun 25, 2020

ecdarreola left a comment

ecdarreola Jun 25, 2020

Choose a reason for hiding this comment

vstumpf Jun 25, 2020

Choose a reason for hiding this comment

Badarosk0 commented Jun 25, 2020

lgtm-com bot commented Jun 25, 2020

ecdarreola Jun 25, 2020

Choose a reason for hiding this comment

vstumpf Jun 26, 2020

Choose a reason for hiding this comment

aleos89 left a comment

aleos89 Jun 25, 2020

Choose a reason for hiding this comment

vstumpf commented Jul 19, 2020

lgtm-com bot commented Jul 19, 2020

Forte22 commented Jul 20, 2020

Badarosk0 commented Jul 20, 2020

hsiaonaiwen commented Aug 4, 2020

Badarosk0 commented Aug 4, 2020

lgtm-com bot commented Aug 7, 2020

lgtm-com bot commented Aug 7, 2020

aleos89 left a comment

lgtm-com bot commented Aug 7, 2020

lgtm-com bot commented Aug 8, 2020

Badarosk0 commented Jun 25, 2020 •

edited