Chaosforge's DRL/D**m RL goes open source

[getter77]

There it is~

https://github.com/ChaosForge/doomrl

[Darren Grey]

Also rebranded from DoomRL to DRL. Hopefully that'll keep ZeniMax at bay...

[Krice]

Also rebranded from DoomRL to DRL. Hopefully that'll keep ZeniMax at bay...

What, so you do have problems with IP?

[javelinrl]

Got a pretty interesting reply on reddit in relation to the timing of the release and the real motives behind it. People were wondering if it was a means of promoting the Jupiter Hell kickstarter or something like that so this is pretty informative:

The C&D was targeted at the website, not the game. Hence I realized that if I push it OS now, I can still push it in it's original state. Even if I have to change it later as a result of a game-targeted C&D, the original source, and hence the original game will still be out there somewhere. You can't stop the signal

Source https://www.reddit.com/r/roguelikes/comments/5gve4x/doomrl_goes_open_source/dawactd/?context=3

[Krice]

I did look at the source code briefly. It's cool that pascal is so object-oriented these days. However I disliked the low amount of comments and they were something like "todo" or some random funny lines. People, learn how to properly comment your source code! It's important. My rule is always comment as if I was some day going to open source the project (which means write comments mainly for other people that possibly don't have skills to read the source code itself). This source code is kind of readable and not that bad, but still it makes a big difference to explain in plain words what each section of code (how large sections are explained is a personal preference) is supposed to do. And also at the beginning of each source file there should be a short summary of what is the purpose of that source file.

[TheCreator]

They say that even better is not to have any comments at all -- that is, the code should be self-explaining. If you don't know/remember what the code does, you need to refactor. I know it's too idealistic for the real life programming, but I often stumble upon totally unnecessary comments like this:

Code: [Select]

int counter = 0; // this counts occurrences of the value in a collection

Wouldn't it be better to just name the variable 'numberOfValueOccurrences'?

I also hate this type of comments:

Code: [Select]

a  = 1; // assign the value '1' to the 'a' variable

What is the purpose of that? Everybody can see that we are assigning to the variable. Even a non-programmer would probably understand this. For some reason, though, people seem to like this kind of commenting the code. Maybe they just misinterpret the principle of comprehensive commenting.

Sorry for this terrible offtopic .

[Avagart]

a  = 1; // assign the value '1' to the 'a' variable

Huh, I remember writing stuff for my colleagues. I needed to comment every line of code, that student on the exam could just read comments if professor would ask for explaination how his code works. This led to really weird comments, like

Code: [Select]

animals = ['dog', 'cat', 'bird'] #create new list
for animal in animals: #loops throught every element of list
    print animal #prints element of list
   

[javelinrl]

I've always thought that good design is more important and more readable than any amount of well-documented bad code but even then I set myself a goal of commenting every class and public or protected method in Javelin - which, thankfully Java IDEs help you do very well and the tooling can generate stand-alone documentation too. Right now though I'm working on a big redesign and changing a lot of my previous code and the sheer amount of work leads me to completely ignoring all the comment warnings on my screen. I plan on getting back to documentation once the work is done, but not before I produce a testing/alpha release.

In the end it will always be more important to create and test your code and get stuff done than to comment everything you're doing along the way - and that could even be counter-productive. This current redesign I've been working on has gone through several iterations even this week - if I were to stop to comment properly on all the things I ended up throwing away it would have just been wasted effort, not to mention I might be more tempted to keep them instead of discarding it and always be striving for the best design.

I'd like to hear what are comments worth to other devs but to me they're always much more of a "can I pass this value as null here' or "does calling this method automatically triggers this other update routine or should I call it manually" and in this sense it is nice to have well commented code but it isn't the end of the world if there is no comment on a function: a person should always be able to go in and write the comment themselves - sure it will take 5 minutes away from what they were originally doing but not more than that unless you're writing big cryptic functions that don't follow any of the standards of modern programming. This one I've come across when creating a small patch for DCSS a couple of days ago is not the worst example out there but it comes to mind due to being "not short", returning several values at once via address pointers instead of separating concerns and having a ton of special cases that are not intuitive at all https://github.com/crawl/crawl/blob/24d37aaa1f410fe53df520af77736b379ba67c61/crawl-ref/source/mon-info.cc#L1323

Meanwhile something like this can be documented by anyone at any time in a minute or so and arguably doesn't even need a comment at all unless you're trying to be thorough https://github.com/crawl/crawl/blob/24d37aaa1f410fe53df520af77736b379ba67c61/crawl-ref/source/mon-info.cc#L1647

Finally, if Javelin was closed source, as DoomRL originally was, I am not sure how much code commenting I'd be doing at all. Probably some, for my own benefit - like just annotate the answer in a comment whenever I have to stop and look at the code about some detail. I must admit though that my major goal with documentation is that if someone ever checks out the project he will find it as friendly as possible and have that be a nudge in the right direction of getting people to join in the effort and also set the same standard if someone does come along for the ride.

[Krice]

They say that even better is not to have any comments at all -- that is, the code should be self-explaining.

They say shit. Yes the code should be readable, but it's not the point. It's still more readable if you explain what the next section of code is doing. It's easier to read that one (preferably) line of comment than read through the source code. Even more than that not all people code the same way. Some programmers have let's say strange ways to do things. In that case it's even more important to write good comments. Some of the examples of bad comments were those you provided, but they have nothing to do with good commenting.

[TheCreator]

They say shit. Yes the code should be readable, but it's not the point. It's still more readable if you explain what the next section of code is doing. It's easier to read that one (preferably) line of comment than read through the source code. Even more than that not all people code the same way. Some programmers have let's say strange ways to do things. In that case it's even more important to write good comments.

No, if you work in a group, you should all code the same way. That's why Java is far better than Perl in big projects, by the way. Comments may help you to read the code, but will not make it much better when you need to modify it -- you'll still have to fully understand that piece of the code, right? Here's an example:

Code: [Select]

i--; // go to the next object

The original line was 'i++', a programmer has changed it to 'i--', but forgot to update the comment. This is a very simple operation, so it is unlikely that anybody gets fooled by this errorneous comment, but it's not difficult to imagine a similar situation with much more complicated code and an outdated, misleading comment. I've seen hundreds of such cases in the production code.

[Krice]

Comments may help you to read the code, but will not make it much better when you need to modify it

Yes it will, because you don't have to read through the entire source code to figure it out. But comments are mainly for reading the source code. So what?

similar situation with much more complicated code and an outdated, misleading comment. I've seen hundreds of such cases in the production code.

That is when we are talking about bad comments. Why do you keep giving those examples? Allow me to demostrate a good comment. One example is a summary at the beginning of a source file. It's so much easier to read it than read the source code that you may or may not be able to understand fast. I've started to write summary myself only recently, but it's something I highly recommend.

Code: [Select]

//Teemu
//flypack.h - Copyright 2008-2016 Paul K. Pekkarinen (paulkp@mbnet.fi)

//Desc: Datatype to contain data of a flying object with the object handle itself,
//direction of trajectory and attempted distance.

#ifndef FLYPACK_H
#define FLYPACK_H

class Game_Object;
class GO_Creature;
class Level;

class Flight_Package
{
private:
...

For summary I've added 'Desc:' so you can find all summaries with that and find out what the source code is doing file by file.

[javelinrl]

Comments may help you to read the code, but will not make it much better when you need to modify it

You're wrong here: a comment is as much of a contract as an interface (in the programmatic sense). If the comment says that "this method will never return a null result" it sure is helping you change the code because if you start returning nulls then you're 99% likely to cause segmentation faults or null pointer exceptions somewhere else. The same goes with any other information in the comment.

Another example: if there is a slow cloning function and the comment says it's a a deep-clone operation, you're probably not going to make the mistake to rewrite it as a faster but shallow-clone operation - unless, of course, you're willing to change all the other affected areas of the code as well since this is a pretty big change in any project that benefits from cloning. In this way the comment is as important and helpful as the return type of the function or it's parameters signature.

[Darren Grey]

Functions should be commented to describe what their overall point is. Otherwise buggy code won't make any sense to anyone. Even if the code is written in a very readable way it's no good if you make mistakes in the code iself.

But let's be realistic, in pet projects like these built up over many years both the code and the comments are likely to be heavily flawed. We should only be surprised by the fact that there are comments at all

[Krice]

Here is another example of how I would comment code. This is again from Teemu. When you comment like this you don't have to read the code, only comments, to find out what the code is doing.

Code: [Select]

void T_Command::Show_Keyboard_Code(int x, int y)
{
//construct key and command names to char buffer 'ot' to display it
char ot[16];
int p=0;
const char ac=cmd_data[cmd].actual_key;

//get the letter of keycode and add possible ~ or ^ index before it
//for alt+ or ctrl+ commands
SDL_Keycode k=cmd_data[cmd].key_code;
const int mk=cmd_data[cmd].mod_key_code;
if (mk==kmCtrl) ot[p++]='^';
else if (mk==kmAlt) ot[p++]='~';

//save the position of letter in 'ot' for possible lowercase change
int sp=p;

//handle special cases to get shorter key names than SDL2 has
switch (k)
{
case SDLK_ESCAPE: p+=sprintf(ot+p, "Esc"); break;
default:
{
if (ac!=0) ot[p++]=ac;
else p+=sprintf(ot+p, "%s", SDL_GetKeyName(k));
}
break;
}

//lowercase SDL keyname for letters (which for some reason are uppercase in SDL2)
if (k>=SDLK_a && k<=SDLK_z)
{
//..but only if it's a lowercase command (shift not pressed)
if (mk==kmNone) ot[sp]=(char)tolower(ot[sp]);
}

ot[p]=0;

//draw the underline between letter and command name
const int xa=n_str->Get_Line_Width(ot);
const int w=64-xa;
const int fw=n_str->Font_Width();
const int fh=n_str->Font_Height();
SDL_Color scol=t_gui->guitheme.Get_Window_Border_Color();
const int px=(x*fw)+xa;
const int py=(y*fh)+fh;
for (int t=0; t<w; t+=3)
t_gui->Draw_Shape(shape::Filled_Rectangle, px+t, py-5, 1, 1, scol);

//show debug commands in green
if (Is_Debug_Command()) n_str->Set_Color(cEmerald_Green);
else t_gui->Set_Font_Color(geWindow_Font);

//show letter and command name
n_str->Write_To(x, y, ot);
n_str->Write_To(x+5, y, cmd_data[cmd].command_name);
}


[javelinrl]

This is terrible in my opinion. I don't often bother writing in-line comments, only function comments. if I were working on your code I'd start by changing this to:

Code: [Select]

/*
* Handles the information from #cmd_data, turns in into a proper letter and associated command and prints it.
*
* Receives x and y coordinates for printing on screen.
*/
void T_Command::Show_Keyboard_Code(int x, int y){
constructkeyandcommands();
        handlemodifiers();
int lowercasecheck=p;
        handlesdlshortkeys();
        handlesdllowerkeys();
drawunderline(); //this and below should go to UI code instead
        showdebug();
        print();
}
Of course this is only a quick adaptation and I didn't consider problems with parameters and such and some of those functions could be merged into a single one if relevant, like handlesdl*. I would also then comment those functions as with the one above if I thought they would be used a lot.

Also from a code design point of view this function is taking way too many responsibilities. I would have totally separate functions to handle the input and others to print it for example. This is clearly a breach of controller/view responsibilities if you're into that sort of stuff.

Sorry if I'm being harsh - other than my personal style preferences your code seems pretty OK and better documented than usual. I've seen way worse code in my lifetime! But that's what happens when you post code and expect other's programmers to react honestly to it 

My main point here is, regarding comments: don't write comments when you can instead create a new function that is easier to read, more cohesive and easier to be re-utilized elsewhere. I hardly allow any functions to be longer than a screen-worth of lines when I write code. Sure, changing context makes functions somewhat expensive to process but I doubt it's gonna have any impact on a RL, especially a turn-based one - and if it does I'm pretty sure you can tell your compiler to inline these functions somehow.

EDIT there is an inline keyword for that http://www.cplusplus.com/articles/2LywvCM9/