*NEW 2015/9/4* Code submission guidelines

Fri Sep 04, 2015 2:41 pm

Congratulations on the decision to contribute to ZDoom! In order to expedite your code submission, please take note of the following guidelines. For experienced programmers most of the following will be obvious. For those of you new to programming, ZDoom is an excellent project to get started working with real world code. You should always keep in mind that you're dealing with a project which has a large history, a large number of users, and relative to most other open source projects a large number of developers.

Several of the following guidelines are written using terminology for feature requests. Most people start contributing by adding new features, but if you are submitting bug patches some of these guidelines are do not apply. You should submit any bug fix patches in a proper bug report thread.

You are required to have a development environment!
There are guides on the wiki on how to compile ZDoom for all supported platforms. You are expected to have compiled AND verified that any code you submit is functional. Do NOT submit code on behalf of other people for any reason. We're all human and we understand if you accidentally forgot to add a file to the repository and that your code may have bugs. As experienced developers we can tell the difference between this and blind programming. If we find that your code quality is consistently bad, you can expect that complex patches from you will be rejected.

Code sanitizer/static analyzer is not an excuse to contribute
If you don't know what you're doing, do not use these tools. We typically find that people who use static analyzers as an excuse to contribute apply the wrong fix. Similarly reporting the errors to us isn't especially helpful as it can become a cat and mouse game. We're not saying they're bad tools, but use them in your own projects first.

We prefer GitHub pull requests
Please consider getting a GitHub account and forking the ZDoom repository. It is not a requirement as you can submit patches via this forum, but it makes our lives easier. Make things easier for us and your code is more likely to be accepted.

Additionally create a thread on this forum for your pull request which include links so that we can easily keep track of both threads of communication. GitHub notifications have a tendency to get lost.

There is no promise of acceptance
While we're more inclined to add features if the work is already done, features will still be evaluated in the context of maintenance and/or performance cost. If there is an implementation design issue, we may provide suggestions on how to improve the design. You are, however, expected to do your own problem solving as this is not a way to force the developers to implement your feature.

Partial solutions will often be rejected as they pose a high risk for backwards compatibility issues.

Commit history is good, but keep it useful
If a patch needs to be completely redesigned. Please discard your pull request and start fresh. We aren't going to ask you to send us only one commit, but the history shouldn't be full of reverts and retries. That history just isn't useful. It is desirable that all commits compile and run as it makes it easier to track regressions.

Additionally, please be sure to write descriptive commit messages for all commits.

Justify your features
Just because your wrote the code doesn't mean you're off the hook as far as justifying why you need the feature. If the only reason we can determine you wrote the feature is "because I can" then the patch will be rejected. A justification is more than simply describing the semantics of your new feature. Tell us what cool things the feature would enable.

If you're submitting a complex feature consider leaving us with a demo wad.

Blend in (code style)
ZDoom's programming style isn't perfectly consistent, but it's consistent enough. Follow the style of nearby functions and blend in. In general here are some rules:
  • Use tabs for indentation. Tab size is 4 spaces.
  • Braces go on a new line.
  • Function/method names are capitalized (camel case). This is SomeFunction rather than otherFunction.
  • Avoid C++ STL. It's not banned, but TArray, TMap, FName, and FString are probably what you're looking for. In general if it's not <algorithms> you probably shouldn't be using it. Basically familiarize yourself with name.h, tarray.h, tflags.h, templates.h, and zstring.h
For the curious the F prefix on classes/structures comes from Unreal Engine.

Stay focused
Bundling multiple features and fixes into a single pull request will lead to it being rejected.

Do not include any code style changes unless you are doing refactoring (unless you're a trusted programmer (see next section) you probably shouldn't be doing refactoring).

Double review
Your feature submission will need to be approved by two developers before being merged. In lieu of approval from a second developer, a trusted programmer can choose to sign off on pull requests including their own. The exceptions to this rule are:
  • It is a bug fix.
  • The feature is deemed trivial.
Trusted programmers: Edward850, Gez, _mental_ (Alexey Lysiuk), Nightfall/Dusk, Torr_Samaho

The people listed above have demonstrated that they have a deep understanding of the ZDoom code base and generally do not require feedback on their submissions. Getting on this list is not a requirement to gain access to the developers forum, which is available to frequent contributors by asking Randi for access.

Re: *NEW 2015/9/4* Code submission guidelines

Fri Sep 04, 2015 3:24 pm

Blzut3 wrote:Your feature submission will need to be approved by two developers before being merged. In lieu of approval from a second developer, a trusted programmer can choose to sign off on pull requests including their own.

So wait, it can be two developers or a one developer + trusted programmer? Sounds like it might be more difficult. There's only 3 official developers, you being one of them, and it's safe to assume you guys have very busy lives, not to mention the trusted programmers. And, well, Randi can just override anyone's saying off the bat, naturally...

Re: *NEW 2015/9/4* Code submission guidelines

Fri Sep 04, 2015 4:15 pm

Correct. One of the three of us have to do the merging, however, I trust anyone listed there to do the second code review. In practice I expect that Graf and I will be the ones doing most of the code reviewing and merging with that rule acting more so that we pass the bus test.

Re: *NEW 2015/9/4* Code submission guidelines

Fri Sep 04, 2015 4:55 pm

Ah okay. Hmm, sounds like it's being done not only to get more input from others, it also gets more people active on it I take it?

At any rate, that does sound like a pretty neat deal. Should also make it much easier on the wait times too, that is, if the trusted programmers agreed to look in on the submissions more often. Or is this assuming they're alright with us asking them to check out our code?

Re: *NEW 2015/9/4* Code submission guidelines

Sat Sep 05, 2015 8:05 am

There hasn't been any sort of agreement between Blzut and the listed people here to respond to requests to review. It's just a list of people who know how to write code, and also me for some reason. :p

Re: *NEW 2015/9/4* Code submission guidelines

Sat Sep 05, 2015 8:11 am

Yeah, I thought so. Still, Graf prefers to wait on other people's responses (at least on mine). So far, I believe mine will hang in limbo for quite a long time unless someone's nice enough to put in feedback on them, and if they think its ready or not. It would be nice to move them along though...

(I'm excluding one of them as randi wants to do that himself.)

Re: *NEW 2015/9/4* Code submission guidelines

Sat Sep 05, 2015 8:20 am

I'm not waiting on some response but for a sign-off by some of the mentioned people. Since none of your recent requests has gotten that yet, they'll have to wait. Of course, if it turns out that nobody cases the rules may prove ineffective. We'll have to see.

Re: *NEW 2015/9/4* Code submission guidelines

Sat Sep 05, 2015 8:23 am

Graf Zahl wrote:Of course, if it turns out that nobody cares the rules may prove ineffective. We'll have to see.

Yeah, that's what I'm afraid of too. I hope that won't be the case. It's one reason some of my older pulls were rejected as well (according to Blzut, lack of interest), AND with some additional reasons.

Re: *NEW 2015/9/4* Code submission guidelines

Fri Sep 11, 2015 11:16 am

Question, mainly towards Graf, about GZDoom only code submissions.

Since GZDoom is more of your baby, does this require still having two developers check it out?

And Blzut, if there is anything specific towards gzdoom features, may want to list them in the first post too.

Re: *NEW 2015/9/4* Code submission guidelines

Sat Sep 12, 2015 8:23 pm

Unless Graf has someone in mind, the two developer rule doesn't apply to GZDoom. My personal interests only extends to the software renderer, so unless it's some Linux compile fix or something like that I have no intention of touching GZDoom stuff.

Re: *NEW 2015/9/4* Code submission guidelines

Sun Sep 13, 2015 8:05 am

Oh, awesome! :D There are more GZDoom features that are planned too, so that's why I asked. Mainly, a few more from GLOOME which will be nice to implement that just deal with sprites.

Re: *NEW 2015/9/4* Code submission guidelines

Mon Sep 14, 2015 9:04 pm

@Blzut3: well, there's an issue you could help with (or at least get it going); while it regards hardware renderer only, it's also Linux specific (well, sdl backend, to be exact), therefore it's not getting much attention.

Ever since ZDoom got ported to SDL2, fullscreen on Linux doesn't work properly if resolution is lower than desktop or rather it does, just not the way it should. I've described the problem here and created a mostly working hack. Mostly meaning it sort of works for me, but most likely would break Windows.

The cause of the problem is obvious - as the raw OpenGL drawing makes no use of SDLRenderer, the screen isn't automatically stretched. To compensate, glViewport calls must get adjusted while fullscreen (my hack is additionally based on on my personal preference of keeping original 4:3 display ratio).

Re: *NEW 2015/9/4* Code submission guidelines

Fri Sep 25, 2015 1:22 pm

What is the proper etiquette when posting code submissions for something that already has a topic in the features suggestion forum? Should we bump that topic with the submission or create a new topic in this forum and link to it in the first post?

Re: *NEW 2015/9/4* Code submission guidelines

Fri Sep 25, 2015 1:25 pm

I usually make a new one in the code submissions page so it doesn't go unattended. I do this because when completing requests, the thread in regular feature suggestions can become lost and buried after a while.

But I only do it when I have something to actually submit or proof of work in progress.

Still a good question.

Re: *NEW 2015/9/4* Code submission guidelines

Sun Oct 11, 2015 8:16 am

I believe it's addendum time...

If you post bug fixes or additions, or post feedback/sign-offs, make sure it's on the forums.