eressea

2 hours ago, xxdem said:

 

My point was that it can be done without a hash map, I didn't meant "implement a C hashmap" that would be stupid, hash maps have a shitload of features you don't need on this case, (one of these features made this bug)

Well, you need to map one ID to another - and as those IDs are sparse, you can't just allocate few megabyte array and store it there. You need some associative container - either red-black map or hashmap... Of course it still can be some sorted (or unsorted) array or something like that, but it will be harder to manage, especially in case there won't be enough allocated memory to store all elements; you would have to reallocate and copy; also insert and delete function would be far from O(log n), however search operation would be O(log n) via binary search if it's sorted.

I don't say C is bad - but std::map is really much more convenient and when used well, it's perfectly safe.

4 hours ago, xxdem said:

That's not quite true about C, it would just be done on a more plain way, you don't really need a complex data structure to do this job, imho.

Writing red-black tree or hashmap in C is really pain in the ass unless you use macros (which I’m trying to avoid/minimize).

It can be done in asm, it can be done in C and it also can be done in C++ - with no or really little performance impact. I find maintaing clean C++ codebase easier than in C or asm - but it’s up to you to pick language you prefer. If written correctly, the result will be almost identical. But it will probably take twice time in asm vs C and twice time in C than in C++. All languages have pros and cons.

1 hour ago, xxdem said:

A good reason on why they should stick with C on such case, this can't happen in C

If they wrote it in C, there would be many more problems than this one (much more code to write).

As for map [] operator - all good C++ programmers KNOW that it inserts element if it's not already present. Operator overloading is good feature - if used sanely - I definitely like writing s == "abc" more than writing s.isEqual("abc") or strcmp(s, "abc") == 0.

Generally speaking, I see C++ bit safer, but you have to use it well. You can write bad code in any language.

Hello everyone,

I'd like to inform you about critical security bug that's present in all widely known NCsoft l2server.exe binaries. If you're using Vanganth, you're definitely vulnerable to this one. If you're using MyExt64, update your server immediately (it's fixed in commit c52b36e).

Quote

 

"Someone has been using my builder account!"

-- Papa Bear, NCsoft and the Three Bears

 

When a player tries to log in, client sends LoginPacket with account ID and session ID. L2server should search for given account ID in std::map<int, int>, compare result with session ID and decide whether it's the correct account and session. However, some programmer at NCsoft made a terrible mistake. Instead of searching in the map like

std::map<int, int>::const_iterator i = map.find(accountId);
if (i == map.end() || i->second != sessionId) return true; // disconnect user

they ended up searching the map this way:

if (map[accountId] != sessionId) return true; // disconnect user

As you can see, if you supply arbitrary accountId and sessionId of 0, l2server will let you in (because it will add std::pair<int, int>(accountId, 0) to the map and then return 0). In reality you can't use any account ID as it's also searched in another std::map, so it works only for accounts that have been logged in since server start - but this is the only limitation.

This bug is really critical, if a player is able to guess account ID of some character with builder that has been logged in since server start, nasty things are going to happen...

I suggest everyone to fix it as soon as possible - you can see the fix in this commit https://bitbucket.org/l2shrine/extender-public/commits/c52b36e8aad518a094774aca49f2b78da7da390b (for Gracia Final, for other chronicles you'll have to find correct addresses)

1 minute ago, verbrannt said:

BTW I can fix it by checking used, but not declared variables, and declare them if needed.

That would be great :) I was thinking about doing the same in the compiler but it would be much harder to do there and also it would be custom change in the compiler... I'd like to keep it all NCsoft way

14 hours ago, verbrannt said:

 

After I've impletented this, I found, that compiler (GF by @eressea) doesn't work properly with {int, int, int, int} syntax of TelPosList property.
I've got many errors during compilation:
 

[d:\vss\globalbranch\work\overseas\ct2_final_0109\program\server\l2npc\fstring.h][112] Can't allocate buffer for fstring

Compiler allocates buffer only for first 38 strings, any next string in OBJ file is empty.
NASC that I'm trying to compile: https://mega.nz/#!WeA1WQKQ!2UxWCjQ8x6Y1IBFlJ7nvj8oIYQM6tKyESGleUmaOPhs

 

 

Fixed - download http://download.l2shrine.com/MyExt64.dll

I'm getting errors about undefined variable talker - it's missing in handler arguments so compiler doesn't see it - first error I get for your AI is

07/22/2018 12:07:04.931, ERROR(780692) : Undeclared variable [talker]

Can you fix it somehow? :)
 

5 hours ago, verbrannt said:

Idk. I have only one occurrence of this variable in my ai.obj files, so can't tell.

Something tells me it should be int - objectID/index of NPC who is boss for the group, but I'm really not sure about that

5 hours ago, verbrannt said:

Also I think boss_pch.txt should be renamed to gm_pch.txt. "Boss" prefix is misleading here.

+1 maybe globalmap_pch.txt... OR maybe parse all of them from manual_pch.txt - that would be the best way

1 minute ago, verbrannt said:

It's already reverted.

Ah, ok :) How about boss_id - that shouldn't be global map ID or should it be?

2 minutes ago, verbrannt said:

It's definitely global map ids :) 

I'm asking because of commit https://github.com/madyanov/nasc-decompiler/commit/87c177446322d3c2688ec3e53c75b216f50d2501

Are you sure that GM_... aren't global map IDs?

EDIT: For example antharas uses global map ID of 10 which is same as gm_antaras_max... On the other side, it's usually defined as a parameter so it can't get resolved nevertheless...

--output option disappeared? :)

EDIT: Ah, it never was there :D

4 minutes ago, zconll said:

There is 3 functions missing from eressea's compiler.

Because they're custom and unique for MyExt64... Maybe add data/gf-myext? :)

1 minute ago, verbrannt said:

BTW tree splitting produces too long paths in Windows, so it can't handle it properly =/
Max path length in Windows is 260 or something.

It works for me on Windows 10 with PHP 7.2.7, I just had to enable Win32 long paths - see https://www.ryadel.com/en/enable-ntfs-win32-long-paths-policy-remove-255-260-characters-limit-windows-10/

Also I've seen you haven't applied UTF-16le decoding part from the patch and still drop all non-ASCII characters, what's the reason for it?

Also, when speaking about NASC versions, the right data directory could be chosen automatically according to the version:
gf = 2, freya = 56, h5 = 60, gd = 73

Just now, verbrannt said:

 

Can you make a PR or patch?

I'm not sure how propagate NASC version to parser.php, if it was there, the fix would be easy:

        if (is_numeric($row[0])) {
            $row[0] = '"' . $this->data->getString($row[0]) . '"';
        }

should be called only if $nascVersion >= 60

Thanks :)

Found another problem that needs to be handled in order to work with H5+ AI correctly. NCsoft apparently moved FString stuff to client side, so now there's

		{1120131; -111092; 232173; -3448; 0; 0 }

instead of

		{"Discarded Guardian (lv20)"; -111092; 232173; -3448; 0; 0 }

in the ai.obj. It should decompile to

		{1120131; -111092; 232173; -3448; 0; 0};

and not to

		{"1120131"; -111092; 232173; -3448; 0; 0};

when NASCVersion is 60+

EDIT: Already fixed in my h5 compiler branch

EDIT2: Also I've removed fstring lookup on 3 different places so it probably won't be just TelPosList but maybe another 1-2 things...

Great, now it works fine :)

Another problem, I get this:

Say("판정식 : " + myself.i_ai6 + " - " + FloatToInt(myself.sm.hp) + " = " + myself.i_ai6 - FloatToInt(myself.sm.hp));

which is wrong (won't compile) because there's no minus operation for strings. It should be parenthesised:

Say("판정식 : " + myself.i_ai6 + " - " + FloatToInt(myself.sm.hp) + " = " + (myself.i_ai6 - FloatToInt(myself.sm.hp)));

It's a minor bug (it's just once in whole ai) but it's bit annoying to have to fix it manually ;)

Updated h5 definitions http://download.l2shrine.com/h5.zip

EDIT: Found one more new problem, if I have 'star_buster_rsk.focus' in precompiled headers, it generates @star_buster_rsk.focus which fails to compile (needs to be quoted like this @"star_buster_rsk.focus")

12 minutes ago, verbrannt said:

Yeah, this warning because of constants in statements like select/if/while.
I think NCSoft uses some sort of preprocessor for AI scripts, that generates such strange code.

For me it seems the ai.obj I'm trying to decompile has already been decompiled (but wrong) and recompiled... So I'll probably have to fix these manually...

Great job, the decompiler works nice :)

Oh, I didn't notice that, well, that's really strange... Also my compiler complains somewhere there: "Stack underflow may be occurred by unary constant" - maybe the select (11)?

Thanks a lot :)

I've found another problem, in some cases, I get results like this:

if (5 == 100 || Rand(100) < 5) {
	if (35003 == 35001) {
		select (11) {
...

Class + data: http://download.l2shrine.com/decompiler-problem1.zip

Sorry for doublepost, one more question, I keep getting av_quest0 instead of myself.av_quest0, any idea what can be wrong?

2 minutes ago, verbrannt said:

Yea, thank you. I will apply your patch.

Only reason I’m using UTF-16LE is to be able to compile result code using your compiler. I though it only supports UTF-16LE.

Yes, it supports only UTF-16LE but I'm using tree structure with UTF-8 files and when I build the AI, I go through the directory structure (so I know which is the correct class order) and join all the files into one big UTF-16LE file, then I let it compile and then I split it again to directory structure (because my extender is able to work with directory structure with UTF-8 obj files).

Maybe I'll share the building script but it needs some polishing and cleaning...

Ah, it compiles my classes without problem :)

May I offer you a patch for reading UTF-16LE input, optional UTF-8 output support and optional tree structure output support? The last one seems to require newer PHP (works with 7.2.7 and enabled long win32 paths for me)

https://pastebin.com/Fir2ZTJd

May I ask what's the reason of ignoring classes guild_master_test_helper1 and public_wyvern?