DDraceNetwork - developer

DDraceNetwork

Development / developer

Development discussion. Logged to https://ddnet.tw/irclogs/ Connected with DDNet's IRC channel, Matrix room and GitHub repositories — IRC: #ddnet on Quakenet | Matrix: #ddnet-developer:matrix.org GitHub: https://github.com/ddnet

Between 2020-06-15 00:00:00Z and 2020-06-16 00:00:00Z

xnj

@Learath2 specialize the override with the condition? or put the definition and declaration for the overridden method together

@deen but that's an ambiguous call, no? The specialized one wouldnt be more specialized

07:33

@deen I was thinking something like https://gist.github.com/Learath2/dfc2c9f51692834a5b4b43cad78e674d but this isn't allowed apparently

GitHub Gist: instantly share code, notes, and snippets.

@Learath2 I finished the thread safe SQL PR. I am also available for questions here.

\o/

09:51

As soon as I find a decent solution for my template issue I'll give it a review

Thanks :)

why use a wordlist?

10:07

why not generate a token

I like a wordlist better

10:08

easier to say and remember IMO

Okay a struct wrapping the function works but that's just ugly. I'll take a look at the PR instead :

Zwelf: Have you considered a base class for the result types so you could handle the result generically?

idk if im blind

10:44

but u are not using prepared statements at all

You are blind

Lots of cool and huge improvements recently, thanks Zwelf & Learath2 especially

@Ryozuki e.g. https://github.com/ddnet/ddnet/blob/9f15acdbee9e73a2e460a89efbe58590a27c096d/src/game/server/score/sql_score.cpp#L1600-L1604

DDraceNetwork, a cooperative racing mod of Teeworlds - ddnet/ddnet

sql::Statement is not sql::PreparedStatement

10:47

just sayin

Are you looking at the same part I'm looking at?

ah yes

10:48

why dont u catch the prepared statement tho

10:48

and also

10:48

other parts of code

10:48

dont use prepared statement

10:49

https://github.com/ddnet/ddnet/blob/9f15acdbee9e73a2e460a89efbe58590a27c096d/src/engine/server/sql_server.cpp#L213

DDraceNetwork, a cooperative racing mod of Teeworlds - ddnet/ddnet

10:49

many things use this function

I think wherever there is user input he is using a prepared statement, but I haven't gotten through it yet

this is not a prepared statement

10:49

and you can cache the prepared statements

10:49

u dont need to delete them

10:49

and create them

10:49

everytime

10:49

https://dev.mysql.com/doc/connector-cpp/1.1/en/connector-cpp-examples-prepared-statements.html

Yes this is one I'm going to suggest, we could prepare all our statements at the start

the point is to reuse it

Well the point is also better sanitization

10:50

One issue with preparing at the start is that the statements become invalid if we ever disconnect from the mysql server

serialization

10:50

would be awesome

10:50

maybe its too much to ask c++

serialization of the state of the server?

of the results

Ah serialization in general

10:51

Yeah not C++s forte

well im sure with c++11 u can do this

10:51

but its ugly

Well no reflection means that you'll have to rely on template magic to serialize

10:52

and most template magic relies on SFINAE which is pretty ugly ngl

well

10:53

u can do the serialization manually

10:53

like

10:53

implement a interface

10:53

FromResult

Why do you want to serialize the result anyway?

looks cleaner

10:53

idk

@Learath2 something like this? https://gist.github.com/def-/6aa3c33b3a8be409166618aa32c1e0cf

@deen did u know Heidelberg is the partnercity of my city Bautzen? kek

kek

@deen hm, are we okay to go from C++11 to C++17?

poggers

chl has the oldest debian

10:58

check if it supports c++17

cant you upgrade tho

we probably can, but someone has to do it

greenthing

11:00

what debian is it btw

11:00

8?

@ᶰ°Konͧsti Yeah, I cycle through Heidelberg every 2-3 days and always see the partner city sign

what is a partner city

https://en.wikipedia.org/wiki/Sister_city (edited)

Sister cities or twin towns are a form of legal or social agreement between towns, cities, counties, oblasts, prefectures, provinces, regions, states, and even countries in geographically and politically distinct areas to promote cultural and commercial ties. The modern concep...

11:02

Chile has Debian 10.4

@deen did you update all hosts to debian 10?

yes

oh nice

11:02

we can drop the cmake hack on chl

and will keep updating them when new debian releases come out

chl was older than the rest for a while

I think I couldn't upgrade because of some problem with the old kernel not supporting the upgrade

11:02

at some point the hoster fixed it

ah

@ᶰ°Konͧsti Bautzen also looks pretty, but too far away for a day trip

if anyone is using a recent kernel and it freezes: https://bugs.archlinux.org/task/66991

FS#66991 : [linux] Sudden freeze on kernel 5.7.2-arch1-1 with amdgp...

Flyspray, a Bug Tracking System written in PHP.

11:04

first time a new kernel gave me problems

11:04

sadge

make a more days trip feelsamazingman

feelsamazingman

feelsamazingman

feelsamazingman

@Ryozuki hm, we are multithreading, I don't think you can share prepared statements like that actually

cant u just lock it

11:08

im not an expert tho

Problem is that the prepared statement is a construct of the connection

the connection pool is safe rn?

Each of our threads gets it's own connection

ah

11:09

well

11:09

but in some parts u still dont use prepared statements

11:09

even if u cant cache them

Yeah, any input that comes from the user should go into a prepared statement

i trhink its better

11:10

u dont keep threads alive right?

11:10

i guess its not a good option

Threads aren't alive yep

11:12

Each sql operation gets a thread, it completes it's job and dies

in an optimal scenario, we'd have a thread pool

11:13

of n worker threads

Also going to ask whether he considered using the engine job pool

The current code doesn't use prepared statements as well. I agree that prepared statements would be better, but I want to finish the thread-safe part first.

11:14

I guess the goal was to get rid of our longstanding thread-safety

@heinrich5991 how were you checking the lowest version of cmake in distros in use?

https://repology.org/ for all distros, debian itself also has a nice package search

Multiple package repositories analyzer

What is the oldest we decided to support anyway?

https://github.com/ddnet/ddnet/pull/2085

Bump minimum required CMake version to 3.5 by heinrich5991 · Pull R...

This would drop support building DDNet with CMake from the following repositories: Distro | Released | Full support | Maintenance support until -------------+----------+--------------+-------...

CXX_STANDARD seems to be there since 3.1

bumping to 3.8

CentOS 6 or 7 seems to be the oldes we support

would allow to use some neat stuff

11:19

iirc

11:19

we dicussed this time ago

I wonder if we even build fine in cmake 2.8 we claim to support

probably not if we don't have CI for it

is there rly ppl on centos 7

CentOS 8 is the newest CentOS

Centos people are odd, I wouldn't be surprised if there were people still using Centos 2

who uses centos anyway

people who don't want to update very often

debian

I guess if your server controls nuclear silos you might consider that level of stability

11:21

debian is bleeding edge compared to centos

but we are talking about a game

11:21

xd

11:21

games most of the time require up to date software

11:21

we dont need to consider nuclear silo ppl

Lets break windowssupport from DDNet

Eh, with teeworlds we do really keep support for old configurations for a long time

11:22

When I did the updater there actually were people still on windows XP

11:22

I hadn't even tested on XP

11:23

@heinrich5991 do you remember why you set -std=gnu++11 manually on mac instead of just relying on CXX_STANDARD?

@Learath2 Oh, I remember that. Every next release had to be downloaded manually for some time ^^'

that was deen IIRC

11:23

try git blame

you seem to be the only one that touched the line

11:25

maybe you moved it from somewhere after deen added it

https://github.com/ddnet/ddnet/commit/7726540de11f2d507c3069a0d9d71535f43e6752

Ah, I can't do CXX_STANDARD 17 until cmake 3.8

11:28

3.8 is a little high

we could work around it

11:28

but I'd guess compilers in these old distributions don't even support c++17

Yeah, hm are we okay with not compiling without c++17 support?

11:38

It's less common than I initially tought

11:38

thought*

doesn't exist on centos 7, debian oldstable, ubuntu 16.04

11:44

also wtf why is ubuntu 14.04 still in security support

is it maybe a LTS release?

I guess enterprises really do want stable software

11:44

yes

11:46

https://en.wikipedia.org/w/index.php?title=Ubuntu_version_history&oldid=961366462#Version_timeline

So do we care that we wont be able to build on those?

I think I'd like to keep debian oldstable support

11:51

that's a good distro to make binary releases from

Okay and since the template wizards in ##c++ didn't come up with anything better either, I guess it'll have to be https://gist.github.com/Learath2/8950c79beb25f36baf152325f2f46d7f

GitHub Gist: instantly share code, notes, and snippets.

11:54

or SendPackMsg getting wrapped by a struct which I can to partial template specialisation on

11:55

maybe bump to c++14 for inline template variables that'd clean it up a little bit

@Learath2 whaty does enable_if?

enable if is an interesting template, if the first argument is false, it'll fail template substitution

11:59

if not it'll have a member type ::type that's the second argument

its like a compile time check?

11:59

ah

a compile time way to disable templates selectively

template syntax gets rly confusing

C++20 concepts really clean this up

Okay it's going to be the gist I linked above, I don' like how the struct looks

That's all the gameplay issues I've found

13:37

now for serverinfo

hey everyone, can someone help me to add files in CMakeLists (DM) (edited)

Uf I forgot I needed to do tokens first

@justabutterflier I initially didn't answer because you wanted it to happen in DM

15:22

I'm generally more interested in answering questions in a public channel, it might help other people as well

15:22

this one: check where similar files are added, and add yours as well

@heinrich5991 0.7 connections start with a CTRLMSG_TOKEN right?

15:25

or is it a CONNECT?

"connections"

15:26

the first message sent is CTRLMSG_TOKEN (edited)

15:26

but the server does not keep state at that point

15:26

the next message is CTRLMSG_CONNECT, at which point it starts keeping state

So I respond to the TOKEN by echoing the token enclosed in a TOKEN message of my own?

yes

15:28

you need to put the peer's token into the header and your own token for that peer into the payload

The client sends a CONNECT with my token in the header, I reply with a CONNECTACCEPT if the token is correct?

@heinrich5991 thanks

Where even is the check for the token?

15:36

Why is there even 4 different token protocols?

I'm done, I can't spend another 20 hours learning the new 0.7 connection protocol with it's 500 moving parts and pieces

15:46

Maybe next week

15:48

What is this even, both send tokens, no one checks shit. BAM connected

@timakro as you have it, I think you can block a slot if you reply with a wrong token

15:58

but god knows if it's an actual attack because I can't read any of this

16:00

Yeah nvm that, funny enough we first accept the connection, then tell the client to sod off if there is no slot for them

16:05

I wonder if there is a version of IOCCC for C++ code that we can apply to with teeworlds

Hah, good that I didn't commit my code, security through obscurity :/

in the token manager @Learath2

16:34

but I think the compatibility layers shouldn't be copied from @timakro or @fokkonaut, IIRC they didn't check tokens

16:34

the compatibility layer code concerning tokens

timakro seems to check it

ok, sorry for that

You are always free to improve upon it after I finish up, I don't see any elegant way to do the lower levels of networking in teeworlds

but you don't introduce IP spoofing vulnerabilities?

Especially when we are supporting 4 protocols essentially

16:40

I hope I don't, at the end of the day only You, oy and maybe god himself can know that

16:41

Maybe matricks but I doubt he'd even recognize most of this anymore

you could cap some packets and check that it looks sane, and modify the token and check that it doesn't connect anymore

There really should be e2e tests for this stuff

yes

intellij-idea-community-edition-2:2020.1... 438.3 MiB

16:52

half a gigabyte

Oh btw @timakro both your calls to UnpackPacket in network_server should be using pData instead of the recvunpacker buffer

16:54

Ah, but maybe you didn't merge deen's recvmmsg patch

16:55

@heinrich5991 IDEs are usually quite large

@Learath2 is it bad to compile through ninja?

17:15

cmake -G "Ninja" ..? (edited)

17:16

or someone

17:16

answer me

When I address reviews, should I add commits with the changes or should I use git rebase to get it "immediately right"?

If you have ninja, ninja is better than nmake

17:24

I just didn't want to make you install more stuff than you needed

17:24

Zwelf: don't rebase to fix past commits, you can just commit more

does someone knows how to add mysql library into cmakelists?

@heinrich5991 i check for tokens too

@justabutterflier you can probably steal it from ddnets cmakelists

@Learath2 I didn't look into the engines thread pool. Where is it located and does the server already use it somewhere else?

Zwelf: CJobPool and IJob are the interesting ones. The only use on the server is for hostlookups but you can check out how heinrich used them for HTTP requests in the client

https://github.com/ddnet/ddnet7/blob/7abcf7b70668955e1ca6a837d8051dc1bb934b68/src/engine/shared/network_token.cpp#L13 it's still using 1 byte tokens right now

DDNet port to TW 0.7. Contribute to ddnet/ddnet7 development by creating an account on GitHub.

17:51

@fokkonaut

@heinrich5991 oh, you mean ddnet7. Yea I dont know :D

@Learath2tried to connect it the same as in ddnet but have some problems

18:59

Embedded image

19:00

the server can't find the libralies

19:00

we make the submodule folder

19:00

with the libraries

19:00

Image attachment

19:01

[CMake]   CMakeLists.txt:249 (set_glob)
1> [CMake]   CMakeLists.txt:1608 (set_src)
1> [CMake] This warning is for project developers.  Use -Wno-dev to suppress it.
1> [CMake] Call Stack (most recent call first):
1> [CMake]   CMakeLists.txt:249 (set_glob)
1> [CMake]   CMakeLists.txt:1693 (set_src)
1> [CMake] This warning is for project developers.  Use -Wno-dev to suppress it.
1> [CMake] Call Stack (most recent call first):
1> [CMake]   CMakeLists.txt:249 (set_glob)
1> [CMake]   CMakeLists.txt:1693 (set_src)
1> [CMake] This warning is for project developers.  Use -Wno-dev to suppress it.
1> [CMake] Call Stack (most recent call first):
1> [CMake]   CMakeLists.txt:249 (set_glob)
1> [CMake]   CMakeLists.txt:1694 (set_src)
1> [CMake] This warning is for project developers.  Use -Wno-dev to suppress it.
1> [CMake] Call Stack (most recent call first):
1> [CMake]   CMakeLists.txt:249 (set_glob)
1> [CMake]   CMakeLists.txt:1694 (set_src)
1> [CMake] This warning is for project developers.  Use -Wno-dev to suppress it.
1> [CMake] Call Stack (most recent call first):
1> [CMake]   CMakeLists.txt:249 (set_glob)
1> [CMake]   CMakeLists.txt:1710 (set_src)
1> [CMake] This warning is for project developers.  Use -Wno-dev to suppress it.
1> [CMake] Call Stack (most recent call first):
1> [CMake]   CMakeLists.txt:249 (set_glob)
1> [CMake]   CMakeLists.txt:1710 (set_src)
1> [CMake] This warning is for project developers.  Use -Wno-dev to suppress it.
1> [CMake] -- Configuring done

19:01

it the cmake output

19:01

the part of output

19:01

63.7 KB

19:01

and the cmakelists

did you try removing CMakeCache.txt etc

yep sure

19:08

i do it after every commit

did you change the paths in cmake files? we have ddnet-liba hardcoded

what do you mean?

you're making a new xpanic mod based on ddnet?

no

19:11

based on vanilla teeworlds

19:11

not exactly sorry (edited)

note that the mysql lib in ddnet-libs often doesn't work anyway

what do you mean?

weird runtime errors

19:12

better to use your distribution's mysql libs

how to do this?

those messages lack the actual warning @justabutterflier

19:14

did you add the source files in alphabetical order?

yeah but it anyway makes noise xd (edited)

barsik will answer

19:19

did you add the source files in alphabetical order? wdym?

19:19

@heinrich5991

19:19

where i need to add

CMakeListst.txt?

yep, it had sorted already @justabutterflier

ok @gerdoe

yeah chiller

Image attachment

CMake Error (dev) at CMakeLists.txt:218 (message):
  GAME_SERVER does not contain every file from directory src/game/server
Call Stack (most recent call first):
  CMakeLists.txt:249 (set_glob)
  CMakeLists.txt:1608 (set_src)

19:36

interesting

Determining if the __i386 exist failed with the following output:
Change Dir: C:/Users/cen940k/source/repos/cen94ok/xpanic/build/CMakeFiles/CMakeTmp

Run Build Command(s):nmake /nologo cmTC_b40c9\fast &&     "C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.26.28801\bin\HostX86\x86\nmake.exe" -f CMakeFiles\cmTC_b40c9.dir\build.make /nologo -L                  CMakeFiles\cmTC_b40c9.dir\build

Building C object CMakeFiles/cmTC_b40c9.dir/CheckSymbolExists.c.obj

    C:\PROGRA~2\MICROS~3\2019\COMMUN~1\VC\Tools\MSVC\1426~1.288\bin\Hostx86\x86\cl.exe @C:\Users\cen940k\AppData\Local\Temp\nm4D12.tmp

CheckSymbolExists.c

C:\Users\cen940k\source\repos\cen94ok\xpanic\build\CMakeFiles\CMakeTmp\CheckSymbolExists.c(7): error C2065: '__i386': undeclared identifier

NMAKE : fatal error U1077: 'C:\PROGRA~2\MICROS~3\2019\COMMUN~1\VC\Tools\MSVC\1426~1.288\bin\Hostx86\x86\cl.exe' : return code '0x2'

Stop.

NMAKE : fatal error U1077: '"C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.26.28801\bin\HostX86\x86\nmake.exe"' : return code '0x2'

Stop.

(edited)

that is part of normal operation

are u sure?

21:34

this is from logfile "cmakeerror"

yes

21:35

this is used to detect the 32bit intel architecture

21:35

caused by this line: check_symbol_exists(__i386 "" TARGET_ARCH_X86_i386)

21:36

it's also in my log of successful cmake

lol

21:36

then why i can't compile

21:36

that's not a full error log

21:37

is it normal? @heinrich5991

Image attachment

Exported 285 message(s)