DDraceNetwork - developer

ubuntu 22 released https://ubuntu.com/blog/ubuntu-22-04-lts-released

Tater

Hey what do you guys think of this pr? I think deen wants some more opinions before merging https://github.com/ddnet/ddnet/pull/4997

I agree with lynn smaller and a label would also be good. But I find this version also good https://user-images.githubusercontent.com/22122579/163871026-3845e192-547d-498c-a8cd-5752dadc8ea6.png You could also solve the label problem possibly by adding to each button a tooltip, like "Set Angry eyes as default eyes", you would have to test how that comes out. (edited)

Thats how it normally looks, it only shows below in the sqaureish aspect ratios

05:28

I think the tooltip is a good idea, adding a label might be hard when its squeezed in between

05:32

The icon size is the same in all the images the aspect ratio is all that changes

Tater

Hey what do you guys think of this pr? I think deen wants some more opinions before merging https://github.com/ddnet/ddnet/pull/4997

maybe put skin prefix to the search filters as popup or smth

07:15

how about have the last selected emote of the emote wheel

07:15

and alawys try that on any server

07:15

thats probably what the user wants anyway

TIL in C++, static variables get destructed after main finishes!?

07:23

that seems insane in the context of multi-threading to me

Rly?

other threads might still use those global variables

I'm the only one who thinks that eye thing shouldnt be added? I like eyes to have a meaning

07:25

@heinrich5991 does rust, or llvm do the same?

07:26

Also is this for just main or for any entry point (embedded?)

I only know of main, no further details

07:26

no, rust does the (IMO sane) other thing

why do you want to kill the main thread without joining the other threads tho

ddnet does that, for example

07:29

other threads might be stuck

07:29

I guess you have to call _Exit in that case

07:30

ah no, quick_exit

07:30

ah no, _Exit, nvm

what threads do we keep open

I don't quite see when else C++ would destruct the statics

when signaling or smth?

07:32

guess there is always a way xd

Jupstar ✪

what threads do we keep open

I guess the sql thread and the curl thread could possibly live past main, if I recall correctly

ok

07:32

we should fix that regardless

why?

i dont think its nice style to leave stuff open u opened

the OS closes it

07:33

faster than you can do. you only slow things down by closing it yourself

u also dont need to deallocate memory

07:33

the os does that

weren't you the person obsessed with performance? ^^

07:33

yea, but at program exit, it's literally doing no useful work ^^

correctness is still the most important thing

it is correct to not free memory at program exit

07:34

and never freeing memory is also correct, probably, it's just not useful because you might run out of it

heinrich5991

faster than you can do. you only slow things down by closing it yourself

It's still probably a trivial amount of time lost if we join all threads before dying

Jupstar ✪

correctness is still the most important thing

oh, then why do u use c++

07:34

07:34

joke joke

@Learath2 only if we have some way to guarantee that they're done

07:35

the score threads in particular have a timeout for joining precisely because they do not terminate

Ryozuki

oh, then why do u use c++

xd a language that prevents logic errros would fix all errors xd

but its true that closing stuff at exit doesnt pose a perfomance impact during the most critical parts of the program

07:35

e.g when playing

yes, if it comes for free, it's fine. or if it's useful for other stuff

anyway, i'd also argument that using static is bad

but we don't have to go to great lengths to do that. "useful for other stuff" might e.g. be "better readable valgrind output"

Jupstar ✪

anyway, i'd also argument that using static is bad

yee

in rare cases u might need them e.g. signaling states

07:36

but else u can always solve it in a better way

my PR introduces a new static

07:36

what's the better way?

07:36

the global logging instance

allocate it on main start

wtf they updated github

and where do I put it? @Jupstar ✪

heinrich5991

the score threads in particular have a timeout for joining precisely because they do not terminate

On timeout, it'll be done. I don't see much harm to the main thread waiting for the sql thread to timeout, especially when we have no idea of the consequences of two servers running with the exact same settings at the same time

does matter use a shared pointer or smth like that

07:37

doesnt

do you expect to pass it to every function so it can log?

we should use unique_ptr and shared_ptr everywhere

What if the dangling sql thread causes some database issue?

isnt that c++11

@Learath2 I think we have a timeout for waiting for sql threads. if you're sure that it isn't needed, remove it

heinrich5991

do you expect to pass it to every function so it can log?

i'd not call it from a global context if u mean that yes

heinrich5991

do you expect to pass it to every function so it can log?

How about a shared_ptr at the very start of the thread?

Client()->LOG or Server()->Log or for specific components Log() as a function inside the class

@Jupstar ✪ sounds impractical and performance sensitive

You can keep the shared_ptr in TLS

i doubt that tbh

I do @Learath2

07:39

passing a parameter to each function does have performance implications

07:39

Client()->Log, Server()->Log, less so

maybe logs can live in Client() and Server() from the engine?

but they're still impractical because not everything knows about Client(), Server()

they would' need to explicitly get the pointer then

logging is a global concern in all languages I know of

07:40

can you show me a project that doesn't have a global logger, so I can see that it is at least a little bit practical?

but then also use the language logging xD

07:40

not a custom one

what about a thread holding the logger and using channels monkalaugh

@Jupstar ✪ but the "language logging" is also just a static variable

07:41

or another case for a global variable:

07:41

in rust, if a file is opened, the stdlib dynamically determines if the kernel has a bug

07:41

if it doesn't, it'll never do the duplicate system call again

07:41

that's a good use for a global variable

ah c++ doesnt support channels in the std

how would you solve that in a better way?

dynamically determines if the kernel has a bug

07:42

how does it do that?

07:42

and what would that bug be

it checks whether the returned file descriptor has the CLOEXEC flag set after opening it with O_CLOEXEC

07:42

there's old kernels that don't know the flag, and apparently some less old kernels where this flag didn't work

ah ok

07:43

but that looks like just holding a bool, doesnt logging have side effects or whathever it is called

@heinrich5991 can't we have the actual instance on heap and only keep the shared_ptr in the static/thread_local?

but the theory was that "all global variables are bad" @Ryozuki

07:44

and I'm arguing that "absolutely not!"

ah ok

the badness comes in the design already, why is the std lib function not part of a class

are u suggesting java?

why would it? less performance

xd

Ryozuki

are u suggesting java?

depends, the OS has to be redesigned for that too

why would you need to do this check over and over again, just because some part of the code needs to construct the file opening object again

07:46

global variables have a performance benefit

07:46

@Learath2 not sure what you mean. currently, we keep a raw pointer in the global variable/thread local variable

they need to check if the variable was allocated in certain scenarious

07:47

e.g. if u use static inside a function

07:47

if we stay with "static"

heinrich5991

@Learath2 not sure what you mean. currently, we keep a raw pointer in the global variable/thread local variable

And where does the logger instance itself get allocated?

yes. let's say "you can use global variables for a performance benefit" @Jupstar ✪

oh no major log rewrite... i can smell conflicts in my 100 ddnet forks :c

@Learath2 the global loggers on the heap, getting leaked

07:48

@ChillerDragon I tried to keep the diff minimal. all existing logging code should continue to work, only the logging initialization is different

pog

continue to work (even better!)

btw is dbg_msg active in release builds? i always wanted to leave some more detail logging with dbg_msg but always feared it might pollute release

because now you can see output from other threads in f1 and rcon

07:49

we could make LOG_TRACE only active in debug builds @Ryozuki (if we add a macro for it)

heinrich5991

@Learath2 the global loggers on the heap, getting leaked

I may have missed the issue here (just woke up), I thought your logger got destructed before the threads were done, if you aren't concerned about the leak, then I don't see any issue here

heinrich5991

we could make LOG_TRACE only active in debug builds @Ryozuki (if we add a macro for it)

ye that would be cool

07:50

it would be even more cool to also enable scoped logs via env vars

no issue. it was just an unrelated post I saw on reddit @Learath2 ^^

so if i want to debug some client specific thing i can get detailed logs on that

07:50

like any decent app

check my PR and comment on the design questions I have @Ryozuki

07:50

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

ill check

@heinrich5991 how does rust go about destructing it's statics? Just using the compilers knowledge of usage?

it doesn't destruct statics

Borrow checker blackmagic

statics live for 'static BASED

that's how I learned about the issue in C++, because someone complained about missing destruction in /r/rust

That sounds more concerning to me. How would you do cleanup in a static then?

"log_log_impl" "log_log_color" honestly i prefer more the classes style instead of c style

07:54

also it would be cool if u used doxygen style docs

commet in the PR pls

so i dont need to parse this in the future :(

anyway, independent of good or bad before we put _Exit now everywhere, keep in mind that other OS rely on global cleanup, e.g. Android It would else just not deallocate there (edited)

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

can you elaborate @Jupstar ✪?

i edited

07:55

i meant not deallocate

I still don't quite get it. if your app leaks memory, it affects the whole system even after the app is closed?

e.g. the java interface calls the .so file lets say libDDNet.so and we use some static variables that require specific intialization objects if its not deallocated it also isnt allocated

07:56

and that creates unwanted behavior for our android port

you mean that if a dynamic library is loaded and then not used anymore at some point, the memory is still leaked?

not leaked, but its still as is

07:57

the global variables are untouched

07:58

and they wont get to initlaize again

Learath2

That sounds more concerning to me. How would you do cleanup in a static then?

by explicitly writing code for it. I guess I don't see a use case rn so I can't really say how to handle it

07:58

yea, makes sense if the dynamic library isn't unloaded

yes but as said, since we rely on the intiialization sometimes we have to unload it explicitly

07:59

else opening the client agian in android will not work

the problem there is code that's not written in mind with something that's executed twice, if I understand it correctly? @Jupstar ✪

i see you use a lot of shared_ptr in the cpp files, but then some functions return ILogger *l as is, is it common practice?

yes

I see

08:03

@Ryozuki where do I see example doxygen docs?

Ryozuki

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

here

08:03

or u mean rendered?

no

doxygen has supports for groups

08:05

so free functions can be grouped

08:05

its nice for new ppl

Ryozuki

i see you use a lot of shared_ptr in the cpp files, but then some functions return ILogger *l as is, is it common practice?

I think I did that because I sometimes need them as std::unique_ptr, sometimes as std::shared_ptr and sometimes as raw pointer

std::unique_ptr is the C++11 way to express exclusive ownership, but one of its most attractive features is that it easily and efficiently converts to a std::shared_ptr.

>

This is a key part of why std::unique_ptr is so well suited as a factory function return type.

ah, interesting

08:08

TIL

https://stackoverflow.com/questions/37884728/does-c11-unique-ptr-and-shared-ptr-able-to-convert-to-each-others-type

Does C++11 unique_ptr and shared_ptr able to convert to each other'...

Does C++11 standard library provide any utility to convert from a std::shared_ptr to std::unique_ptr, or vice versa? Is this safe operation?

comment on the PR with that please, so it doesn't get lost

ok

Once you’ve turned lifetime management of a resource over to a std::shared_ptr, there’s no changing your mind. Even if the reference count is one, you can’t reclaim ownership of the resource in order to, say, have a std::unique_ptr manage it.

08:09

weird, with a reference count of 1, it should be safe to turn back

hmm

ah, I guess it's because multiple threads might have a reference to the same std::shared_ptr

08:10

makes sense, nvm me

08:10

(with rust, you can still turn it back though!

)

:o

08:11

what do u think about doxygen?

08:12

btw we have docs generated every day (or week i dont remember): https://codedoc.ddnet.tw/

08:12

i also started this some time ago https://wiki.ddnet.tw/wiki/Development

Development

This article aims to introduce you into DDNet development, since it's an open-source game, it relies on random people kind enough to contribute to it on their free time.

08:12

if any time u want to add something

I like docs that say something

well the doxygen is just porting the system.h docs

yes

so idk what that has to do here

08:13

I don't like redundant docs that are just restating the function name or the parameter names. I've seen such docs added in the past in projects that tried to increase some documentation metric

Ryozuki

so idk what that has to do here

you asked me what I think about adding doxygen

ah well

08:14

i meant about system.h

08:14

but ye

I think those redundant docs are worse than no docs

08:14

ah

we already had this discussion

08:14

u can have it ur way

xDDDD

❤️

if we agree then