MrAntares / roBrowserLegacy

This is a continuation of the original robrowser. All credits to the original creators and the new ones :)
GNU General Public License v3.0
193 stars 118 forks source link

Client on Packetver 20211103 non-renewal won't pass the calculateBlockSize validation #456

Closed biali closed 1 month ago

biali commented 1 month ago

I had to return this to its original :

if (_value >= 20201007) { blockSize += 4; // hp blockSize += 4; // maxhp blockSize += 6; // sp blockSize += 6; // maxsp }

I am using rAthena with no pincode validation.

https://github.com/MrAntares/roBrowserLegacy/blob/db5b9bb222c1cbaef6678da595e62801a7398d65/src/Network/PacketVerManager.js#L109

redpolline commented 1 month ago

That's odd, given that rathena's source matches ours. https://github.com/rathena/rathena/blob/1818600e5088b01710a8e1c1c53dd53f8074fe1c/src/char/packets.hpp#L37

The commit that added that to rathena was made two years ago. Prior to that it matches your change. What version is your rathena server running?

biali commented 1 month ago

Ii dont’t have the exact version right not (will update this with that) but it is from about Jan/2024.

Let me try harder on my end… maybe it is a side-effect of something I changed server-side.

Samuel23 commented 1 month ago

image

image

image

tested with this server (one of the server posted in discord), this one is set at 20211103 address: "ro.choigame247.online", socketProxy: "wss://ro.choigame247.online:5999"

biali commented 1 month ago

our list length is 175. not sure that would help =x

Samuel23 commented 1 month ago

_RENEWAL always return false in this file

added console.log("_RENEWAL:", _RENEWAL); in any part of this file and you will always receive false.

seems Configs.get('renewal') still undefined upon creation of _RENEWAL variable (which makes it always false)

and in rathena: image

PACKETVER 20211103 is considered as PACKETVER_RE (which means it really has 175 blockSize)

redpolline commented 1 month ago

Hmm....

The versions I have are:

https://github.com/rathena/rathena/commit/a0bc2c0c3d85afb311607c42b11216fababd80d6 (For packetver 20210406, made Mon Aug 1 10:45:40 2022 -0400)

and

https://github.com/rathena/rathena/commit/a5c939bec95ceab467e7fedc530460e4a70e7138 (For packetver 20130618, made Fri Aug 9 23:33:43 2024 +0200)

Both of those rathena versions work with the current master branch on my end. Screenshot_2024-08-18_14-00-44 Screenshot_2024-08-18_14-01-46 Screenshot_2024-08-18_14-02-18

In the case of the older build, it lacks the changes to the packet structure. Matching the changes that @biali made.

redpolline commented 1 month ago

_RENEWAL always return false in this file

added console.log("_RENEWAL:", _RENEWAL); in any part of this file and you will always receive false.

seems Configs.get('renewal') still undefined upon creation of _RENEWAL variable (which makes it always false)

and in rathena: image

PACKETVER 20211103 is considered as PACKETVER_RE (which means it really has 175 blockSize)

Which file?

Although that sounds like issue #428....

Samuel23 commented 1 month ago

@redpolline

https://github.com/rathena/rathena/blob/1818600e5088b01710a8e1c1c53dd53f8074fe1c/src/config/packets.hpp#L22

hmm i think can just revert this file to previous but just change 20201007 to 20211103

20211103 has blockSize 175

or change how _RENEWAL is defined with correct value, as it just defaults to false as of now

redpolline commented 1 month ago

So there's multiple packets.hpp files in rathena..... Screenshot_2024-08-18_14-31-07 Screenshot_2024-08-18_14-31-33

Problem is that PACKETVER_RE isn't checked when rathena builds it's CHARACTER_INFO struct. It's only looking at the raw version number. So who's wrong here?

As for changing _RENEWAL, I'd be afraid that we'd break it for anyone using a non-renewal server. As that would autochange any roBrow server that didn't define _RENEWAL in their config expecting it to be false by default.

redpolline commented 1 month ago

20210406 Windows client connects to my 20210406 server. screenIshimuraServer004

If that block size was supposed to be different, shouldn't the windows client refuse to connect?

redpolline commented 1 month ago

Idea: https://github.com/MrAntares/roBrowserLegacy/blob/db5b9bb222c1cbaef6678da595e62801a7398d65/src/Engine/GameEngine.js#L241

That check doesn't include a line for checking the RENEWAL flag or PACKETVER. It only checks for a different server address or port.

I wonder if it's possible that their global config in index.html doesn't set _RENEWAL or PACKETVER but does in their server definition? If so, that might explain it. As loadFiles() doesn't get called in that case, it would still being using a DB and Client that had the conflicting global config.

redpolline commented 1 month ago

OK, I've pushed a commit in my fork that will set the _RENEWAL flag in LoginEngine.js if the selected server config differs from the current setting in PacketVerManager.js.

@Samuel23 @biali Can you guys test that commit and see if it fixes the issue?

Samuel23 commented 1 month ago

@redpolline

So in rathena, PACKETVER is set at config/packets.hpp

PACKETVER_MAIN_NUM - represents KRO_Rag exe PACKETVER_RE_NUM - represents KRO_RagRE exe

so this clients versions doesn't represent anything whether a server is renewal or pre-renewal, because even pre-renewal servers can use those client dates if they want to

so in char/packets.hpp

the line:

if PACKETVER_RE_NUM >= 20211103 || PACKETVER_MAIN_NUM >= 20220330

int64 hp;
int64 maxhp;
int64 sp;
int64 maxsp;

else

int32 hp;
int32 maxhp;
int16 sp;
int16 maxsp;

endif

This will be true as long as the client date is 20211103 and above (no need to check whether if renewal server or not, only client date matters here)

so in this line: if ((_RENEWAL == true && _value >= 20211103) || (_value >= 20220330)) { blockSize += 4; // hp blockSize += 4; // maxhp blockSize += 6; // sp blockSize += 6; // maxsp }

we could already ommit _RENEWAL == true, we just need _value >= 20211103

so in your sample: https://github.com/MrAntares/roBrowserLegacy/issues/456#issuecomment-2295356328

20210406 will fall into blockSize 155 because it is not yet reached 20211103

if it is a typo and you meant 20220406, there would be no problem as well, because the later check ( || (_value >= 20220330)) ) will result to true which makes the blockSize 175

to not make things more complicated, _value >= 20211103 would be enough checks in here

redpolline commented 1 month ago

Sent a pull request for the PACKETVER bug.

Thanks for the explanation @Samuel23. :+1:

Boblee-D-Dev commented 1 month ago
Screenshot 2567-08-19 at 10 06 11

@redpolline btw, Pincode it's could be start with zero or another format with digit :)

alisonrag commented 1 month ago

fixed at #457

biali commented 1 month ago

It seems its all sorted!! Thank you guys!!