Contributor instructions

[DarkDragon]

I've taken a stab at drafting instructions here: https://github.com/ArmageddonGames/ZeldaClassic/blob/cmake/CONTRIBUTE.md
Gleeok and others, please let me know if you have any comments.

[ZoriaRPG]

I've taken a stab at drafting instructions here: https://github.com/ArmageddonGames/ZeldaClassic/blob/cmake/CONTRIBUTE.md
Gleeok and others, please let me know if you have any comments.


...so.. What do you want/expect us to do, with somethin that has already undergone months worth of patches, improvements, feature additions, and other changes? How exactly do we submit that, incrementally?

Realistically, that would be mental. We could upload each stage of our betas (46 thus far) into their own git repo, so that you could track each change, and patch the cmake source with all the new stuff that works; but doing brand new incremental patches would take an excruciatingly long time, particularly as some things are intertwined at this point, and require one-another for support.

Thankfully, we've kept a decent changelog to go with it. I personally hate git, but that's a side-issue.

Also...


We also maintain branches for each version of Zelda Classic that we have previously released. Some bugfixes are back-ported to these branches.

Does this mean that you plan to put up the source for all the old versions?

[DarkDragon]

Hopefully a lot of the small changes are easy to rebase. For the super intertwined stuff, yeah, there may be a lot of work involved, sorry :shrug:

I know that the current state of affairs is in large part our (i.e. the ZC devs) fault for not keeping up with pull requests over the past X years, which caused your codebase to diverge from the main repository. But it is what it is. I'd have to get hit by a bus before allowing a huge mess of changes and rewrites into the main repository without reviewing and testing each feature independently.


Does this mean that you plan to put up the source for all the old versions?

No. I don't plan on supporting anything except the previous stable build (currently, 2.50). If other devs want to maintain bug fixes for even older versions, though, that is fine with me.

[Grayswandir]

What if I just want to, like, reorganize the code and add in comments and stuff, without changing any of the actual logic?

I've been banging my head against the parser recently, and I'd like to go through and add comments to things, change the ordering in the files so it's more consistent among all of them, rename some of the poorly named classes/functions, and in general make it more readable.

Also, minor structural changes, like getting rid of the "Clone" visitor and just adding clone() functions to all of the AST nodes. (This'd drop it down from 3 lines with several typecasts into a single function call.)

Third thing - can I use const? I noticed y'all didn't use it, even where it made sense to. Is that just lazy, or stylistically you don't want to use it, or what? It's hard to tell what's a choice and what's just an artifact from 10 year old code or whatever.

[ZoriaRPG]

Hopefully a lot of the small changes are easy to rebase. For the super intertwined stuff, yeah, there may be a lot of work involved, sorry :shrug:

I know that the current state of affairs is in large part our (i.e. the ZC devs) fault for not keeping up with pull requests over the past X years, which caused your codebase to diverge from the main repository. But it is what it is. I'd have to get hit by a bus before allowing a huge mess of changes and rewrites into the main repository without reviewing and testing each feature independently.



No. I don't plan on supporting anything except the previous stable build (currently, 2.50). If other devs want to maintain bug fixes for even older versions, though, that is fine with me.

So, does that mean that you want an individual commit for every single ZScript variable, because again, that's madness. I can see doing them in groups, by class or application, but one at a time is just impractical.

You might want to remove the comment about older codebases.

FWIT, I'm probably not going to touch this until the new script drawing that Gleeok wrote is installed in it; unless it is installed now. I do want to to keep the same ZASM IDs on everything that we've added, so that compatibility loss is not a concern, too, so I may need to rearrange the new script drawing commands to fit into the tables that I'm using, as there is no build yet that calls any of those.

On the subject, why the flidd is the list of defines in Bytecode.h not an enumerated list?!

[DarkDragon]

Groups of variables are fine. (Especially if they are simple getters/setters) but please don't add dozens of functions at once, plus C-style block comments, plus function pointers, plus...

Also there has been disagreement in the past about which variables are worth adding and which are not. If you send a pull request with a large group of variables, you might be told to break it up further.

[DarkDragon]

What if I just want to, like, reorganize the code and add in comments and stuff, without changing any of the actual logic?

I've been banging my head against the parser recently, and I'd like to go through and add comments to things, change the ordering in the files so it's more consistent among all of them, rename some of the poorly named classes/functions, and in general make it more readable.

Also, minor structural changes, like getting rid of the "Clone" visitor and just adding clone() functions to all of the AST nodes. (This'd drop it down from 3 lines with several typecasts into a single function call.)

Third thing - can I use const? I noticed y'all didn't use it, even where it made sense to. Is that just lazy, or stylistically you don't want to use it, or what? It's hard to tell what's a choice and what's just an artifact from 10 year old code or whatever.

All of these sound fine.

Const-correctness is fine in principle but in practice it's like the magician's handkerchief trick: you keep pulling and pulling and pulling and you never quite finish pulling out the handkerchief. If you send a pull request with const-correctness fixes AND it doesn't break anything on Gleeok's old Microsoft compiler AND it doesn't require a bunch of const_casts to deal with Allegro or other external libraries, then sure, that's fine.

[ZoriaRPG]

Groups of variables are fine. (Especially if they are simple getters/setters) but please don't add dozens of functions at once, plus C-style block comments, plus function pointers, plus...

Also there has been disagreement in the past about which variables are worth adding and which are not. If you send a pull request with a large group of variables, you might be told to break it up further.

Has anyone in recent history, reviewed the change logs or ZScript files to note what I added to this build; or is this just speculative based on ideas I've thrown around? Why not look at the files:

Here is a compiled list (http://pastebin.com/9ciakamG) of the additions, in short form.

To be honest, I just don't feel that this is worth touching at all until the following occur:

(1) Saffith signs off on it. Top prirotiy, that, as his decisions have shifted this back and forth several times, up until this fork, which is based on what he said would be the master branch. I don't think it is worth devoting time to it until this happens.

(2) The latest bugfixes to 2.50.3 are implemented in it.

(3) Gleeok's script drawing changes are implemented. He changed several factors in the way that this works, and until those changes are solidified, building on it is pretty wasteful. Note that DrawBitmapEx() will NOT work without at least some allegro 4.4 components. I hacked in some drawing routines from 4.4 (into 4.2, and they still run on the old libs) to get it to work at all. For proper, and full implementation though, 4.4 is mandatory. At present 70% of the supported modes are disabled by default.

(and D) There is public disclosure on who is in charge.

I would also like to see a core dev hierarchy posted visibly. All the discussion on this branch should have been public, IMO. Keeping it dark and then throwing it up there seems to me, to be in bad taste for an open source project. Everyone involved should have had in out, and while I don't have objections to the refactoring, many questions remain unanswered, and this still does not seem finalized. I haven't a clue who was involved, other than you, at present, and who approved the branch.

Because of what has happened in the past, I'd like to see Saffith and Gleeok mark this as approved before I even think of touching it... This is nothing against you, but rather, to prevent doing this a fourth, and a fifth time. This is already my second round of fun, and the new CMAKE branch will make the third. I specifically took all of December off to work on this, because it was (supposedly) finalized; during which time I have often elected to not sleep, just to keep on focus, as to get something usable completed.

I do not have time to reimlement the same things repeatedly, and I would appreciate if the core devs would go over the changelog, and the short list of new stuff, at the least, and tell me in advance if anything is not going to be included. This could well make the difference between working on the core project, and just going me own way with it. That was always in the cards, from the onset.

I certainly do not wish to waste me time makng changes that won't be included.

It would also be nice to see some documentation on the changes; particularly the benefit and drawbacks of what the refactoring entails, in summary form.

[Gleeok]

ZoriaRPG: I will work with you to get those ZScript changes done, in a simple FIFO order starting with the things that were already in master.


[edit] Quick note on 'const': const does not affect compiled code, but should be used on procedures as a form of self-documentation. eg.; memcpy(void* restrict dest, const void* restrict src, size_t n); implies that dest may/will be modified and src will not. But the compiler cannot optimize 'const'. Also "foo::method() const" is correct. All other uses of const IMO can be just as stupid as 'mutable', which is a whole other thing that's likely to start a pointless why c++ sucks or doesn't suck debate, so we should avoid that.

[DarkDragon]

(1) I think it is an excellent idea for you to discuss with Saffith what changes he thinks are valuable to make to the ZScript system. In fact I would like to be involved in those discussions as well. It is good to have such discussions before anyone does any more coding.

(2) Those should already be in (if they were in 2.50.x).

(3) Merging in Gleeok's changes from the current-master, and adding Allegro 4.4 (again, assuming this fixes more hardware issues than it creates) are both high-piority items for the near future, as I've said in the past. I will work on this myself once the build system and repository is sorted out.

(4/D) There is no hierarchy. You can see the list of people with commit access on GitHub, and War Lord decides what binaries are available for download on zeldaclassic.com, and whoever is in charge of PureZC does likewise over there. That's it in terms of formal structure. Informally Gleeok is the current lead developer, and I will defer to his final decisions, but for the most part development works by consensus. If one developer goes rogue and runs amok, the rest can fork the repository and keep working somewhere else; such is the nature of open source.

This does mean that there is a constant risk of miscommunication and developers working at cross-purposes. It also means that decisions can be slow to make and changes difficult compared to a corporate setting where some manager hands down orders from on high. For example, I am currently trying to convince Gleeok that using CMake instead of manually-curated Makefiles is a good idea. If I'm not successful, the whole CMake thing will get rolled back. You are certainly wise to wait and see how things fall out before investing a lot of your time.

My best advice to you is to pick one small but important change you would like included in the official ZC release, and (once the repository is sorted out) make a pull request containing that change (and only that change). One of us will review and accept it. Once you can see for yourself that the change is in the official ZC source, start working on the next feature. This safeguards your own time, and also makes it easier for us to check your work: win-win.

It sounds like you are hoping one of us will pick through your fork and check all of the tens of thousands of lines of changes and personally approve them, and copy them into the official source. I hope you understand why this is probably not going to happen. I *do* want to work with you and I *do* want to make sure your useful and valuable features find their way into the ZC source, but I also have a responsibility, which I consider even more important, to ensuring that ZC remains stable, maintainable, and backwards-compatible. I will help you, but you have to work within the collaborative-development system.

And for the record, I am really sorry that you are in this position. This situation, where you have a huge divergent fork, should never have happened, and should never be allowed to happen in the future. But the only reasonable way forward now is to go through your changelog and apply patches one useful feature at a time.

[DarkDragon]

It would also be nice to see some documentation on the changes; particularly the benefit and drawbacks of what the refactoring entails, in summary form.

See this post: http://www.armageddongames.net/showthread.php?97781-Building-on-Mac-and-Linux&p=912977&viewfull=1#post912977

[Dimentio]

Okay, one thing that's been eating at me at the past little while: who is officially considered a developer? Is anybody who has contributed to ZC a developer? Is it only people who have contributed a huge amount of time to developing ZC? If not a developer, then what are they? A contributor? I'd like to know where the line is between "official" content, and forks.

Also, if we are indeed moving to CMAKE, would that make any work done using MSYS obsolete?

I apologize if you have answered these questions elsewhere. I'm not exactly what you'd consider as having a good attention span, or whichever the term is. :P

[DarkDragon]

(3) Gleeok's script drawing changes are implemented

Can you point me to exactly which drawing changes you are talking about? Are these changes he already committed somewhere? Or planned but has not yet finished?

[DarkDragon]

Okay, one thing that's been eating at me at the past little while: who is officially considered a developer? Is anybody who has contributed to ZC a developer? Is it only people who have contributed a huge amount of time to developing ZC? If not a developer, then what are they? A contributor? I'd like to know where the line is between "official" content, and forks.

The official repository is at https://github.com/ArmageddonGames/ZeldaClassic. I suppose people with write access to it are the "developers" and people without, but who have submitted patches to the code, are "contributors." I leave all decisions about granting write access (i.e. turning contributors into developers) up to Gleeok, but I imagine two important traits are 1) being competent enough around the ZC code that we trust you not to break existing features, and 2) getting along well enough with the existing developers that we can communicate and collaborate productively without killing each other. ;)



Also, if we are indeed moving to CMAKE, would that make any work done using MSYS obsolete?

There is no reason CMake cannot create a Makefile compatible with MinGW/MSYS, if that is what you're asking. I am happy to help with that. I do not have MinGW installed on my machine, though, so wasn't planning on setting that up unless/until someone specifically asked for it.

[Gleeok]

Can you point me to exactly which drawing changes you are talking about? Are these changes he already committed somewhere? Or planned but has not yet finished?

New features, so they are in the trunk, not in the 2.5 stable branch. I'll put them back in the new dev branch when we get this other stuff sorted out.

[ZoriaRPG]

New features, so they are in the trunk, not in the 2.5 stable branch. I'll put them back in the new dev branch when we get this other stuff sorted out.

The sooner, the better please. I would rather not rewrite stuff a third time. :p

I should at least update the commands list first though, so that they use the correct, reserved registers.