opentomb / OpenTomb

An open-source Tomb Raider 1-5 engine remake
http://opentomb.github.io/
GNU Lesser General Public License v3.0
1.39k stars 145 forks source link

General project development discussion #252

Closed Lwmte closed 8 years ago

Lwmte commented 9 years ago

This thread has eventually evolved into discussion about general project development and future.

As you already know, we have an ongoing OpenGL 2.1 vs. 3.2 discussion.

Pros for 2.1 - Almost universal compatibility, even with older videocards and drivers. Cons for 2.1 - RenderDoc is not compatible with OpenGL 2.1 (but it's also not compatible with over-3.2 contexts - surprise!)

Pros for 3.2 - RenderDoc compatibility Cons for 3.2 - Minority of users won't be able to use the engine. Not compatible with GLSL used in 2.1 (at least in OS X).

Let's try to make a decision here.

cochrane commented 9 years ago

I vote for 3.2. My reasoning: As the game becomes more complex, we will want to implement more complex features and so on, which will require a switch to 3.2 some day anyway. Might as well do it now.

But this isn't a point I feel strongly on. I believe we should not use features that we can't use in 3.2, but as long as that is ensured, I can live with 2.1 as well.

I thought for a second about using the shader precompiler so we can write the shaders in a way that allows them to be used on both, but that seems like an awful lot of effort for something that brings us no benefit whatsoever.

Regarding debugging: That's actually much worse with 3.2 on OS X. I do hope they'll fix that eventually, but I don't see it as a blocker either way. To debug the texture atlas, we can just write the data to PNG files or something. For most other things, the debugger gives hints, but rarely full solutions anyway.

stohrendorf commented 9 years ago

I don't want to work on an application that requires hacks to be actually debuggable.

vvs- commented 9 years ago

Not compatible with GLSL used in 2.1 (at least in OS X)

Can't say anything about OS X. But on Mesa (Linux) it's fully compatible right now. I've just tested it on a software renderer.

There are other tools for debugging besides RenderDoc. Actually, I feel strongly against the dependence on a platform specific tools (and not free btw).

vvs- commented 9 years ago

Actually, I'm not qualified to vote here because I have no choice. While I still can revert shaders at present time it could eventually prove to be too difficult.

godmodder commented 9 years ago

I am not voting here because I work on the editor and not the engine, so you guys can do whatever the heck you want. But seeing that I might contribute later, I would like to give advice from my 5+ years of professional experience with these issues in OpenGL. The points I would like to make are:

1) Maintaining backwards compatibility is a MAJOR pain in the ass and most of the time it was THE reason I was pulling my hair out during a project. It is one of the most time-consuming activities and something an open source project with few participants wants to avoid. 2) You WILL need a decent debugger later on in the project when advanced rendering comes into play. OpenGL is a state machine and tracking down these wrong states is so much easier with the right tools.

Lwmte commented 9 years ago

I personally have no preference for 2.1 or 3.2, cause my hardware supports even 4.4. Our survey also says that 90% of users have modern hardware. It's really a pity that certain folks out there can't launch our engine because their hardware is too old.

But I also thought about something else. Currently we have OpenTomb in pre-alpha stage, i. e. we still have no NPCs, no menus, no cutscenes support, collision is faulty... I think we need at least one or two years to make it stable. So anything may happen in these two years. First of all, market share of 3.2-capable hardware will be even larger than now. Second, maybe someone will write some kind of system to plug in various types of graphics APIs into it. Third, maybe Vulkan will be so cool and backwards-compatible that everyone will use it.

So what's the point of arguing about API versions which will be already outdated by the time engine reaches its release stage?

vvs- commented 9 years ago

Exactly. I've already bought the card which supports OpenGL 3+ and now it's obsolete because I've upgraded my computer. I don't want to spend money on obsolete technology every year. I have a better applications for it. I prefer to wait until I get something real in return.

stohrendorf commented 9 years ago

OK, I have to say that now before I explode, because I have been very calm in all these discussions. And I know that most of you (if not everyone) won't be happy after reading this.

  1. I fucking hate the idea to use only OpenGL 2.1, as I foresee that OpenTomb won't be able to provide all these perfect, great, awesome, amazing features it promises. Go ahead, promise AAA graphics and features, and stick with a deprecated OpenGL version. And as the features get more complex, you can't really debug the graphics, because most tools (free and commercial) struggle debugging old OpenGL versions. We have. what, about 1% of the users that don't have a 3.2-compatible hardware? Yeah. I can build a computer that doesn't support OpenGL at all. Can you please make OpenTomb run on that thing, because I am a user?!?
  2. I fucking hate the idea of the "OpenTomb Build Micro-Universe", i.e. including OpenAL and Bullet in-tree. I have tried to create the cleanest solution I can live with in #247, but that pull request seems to be fully ignored. You want to create good-quality software, but you try to circumvent even the dumbest things that can happen by pre-packaging (and maintaining!) external sources. The easier it gets to set up a build environment, the easier dumb developers will arrive. OpenTomb has reached a code size and complexity that you don't want dump developers here. And if a developer struggles at building a local version of the requirements on a system without a package manager... well, it took me a day to set up the MSVC environment, with building all libs from the sources. On Mac OS X, I assume it's the same situation. Require all devs to remove system provided OpenAL and Bullet libs, just to make sure that never ever any libs mixing occurs? Hell, you have to place a bomb next to the build system to make this happen.
  3. OpenTomb is still years away from being released, but you are already showing great promo vids, screenshots, features, etc., from which a user concludes that it's nearly finished. It's not. The code is a mess compared to the project size. It should be cleaned up, reorganized, converted to proper C++, and generally be feature-frozen until it's clean.
  4. There is no proper review process. Pull requests are made by someone, and then merged by that same person without a proper review whether the code is OK. If it's not merged by the requester, crappy code gets merged. Happened with the TRS3 request and with my own first pull request. I had it there to go through that process, and also put a huge warning into the description that this was experimental code. What happend? It got merged with the comment "let's see what happens".
  5. Bugs, bugs, bugs. Nearly every day new bug tickets are created, but nearly never they get solved. There are over 80 tickets open right now, and I just want to see some of them closed, or being worked on. This doesn't happen. OpenTomb isn't making progress, it's making "backgress". The ticket growth is in no relation to the feature growth. I'd really like to work on some of the bugs, but I can't, as the code is control-flow-spaghetti in my eyes that I don't understand enough to have an idea of possible side-effects when I change something. There's no abstraction/modularization. And I don't want to modularize the code yet, because I don't want to create new tickets.
  6. The whole project is completely unorganized. There is not ticket handling progress (nobody feels responsible), there is no review process (all just gets merged without checking for quality guidelines that even exist yet), nobody seems to respect that "every new feature on a branch"-rule that was discussed a while ago.

I'm not perfect, I know, and I broke some of the rules myself. I just had to say it all.

vvs- commented 9 years ago

Calm down. It's just a game not a life of yours.

I'm actually agree with much of the critic here. But it's a voluntary amateur project. Don't beat it too hard just because nobody pays the developers and they do it in their spare time. And that time is precious.

vvs- commented 9 years ago

And I must add that nobody reached that stage before. It's a true achievement. Anyone who would try to repeat that will know what I mean.

stohrendorf commented 9 years ago

This project is too large to not have these rules. It needs these rules, because it is not an amateur project anymore. Amateur projects usually have 2-3 people involved with at most 20k lines of code. This project has about 10 developers and a size of about 100k lines of code. I made #146 for organization purposes, because it's not clear (if at all) who is working on which ticket. We have many tickets that are nearly-duplicates, because they probably have the same cause. And nearly every time I try to get rid of old/deprecated/discouraged things, I get some rants, with reasons pointed out that I mostly know are wrong, otherwise I hadn't tried to get rid of these things. I'd like to see some organization, review process, code quality (not style) guidelines, etc etc. Until this is done, the project is doomed fail.

Lwmte commented 9 years ago

Ok, let's just stop working on the project, right?

I can also say many things to you. I spent whole summer this year by the computer, while I could travel to some cool place like rafting or mountains. I wasn't educated as a programmer, my speciality in university was linguistics. I had very poor knowledge of math. Because of this, I'm not trying to look as a "professional" here. Yes, I'm trying to promote OpenTomb, because without this promotion, I bet you wouldn't know it even existed, or maybe stumbled onto it when it was very well abandoned, right?

Currently, OpenTomb is the most advanced thing TR community ever created, not counting NGLE/TRNG, which was indeed written by the single person, yet is just a patcher (no matter how big is the worship of Paolone on TRF).

Now you arrive here and say that everything we do here is crap, we poorly write our code, we don't manage tickets, there are lots of bugs, etc. etc. Considering that it was your pull request which broke almost 100% of our code and provoked creation of tens of tickets in issue page. We gave you rights to contribute right to our repo because you was trusted that you will fix your own bugs. Unfortunately, all major bugs introduced by you weren't fixed by you. It was stltomb who fixed rotations/positions. It was me who fixed texture atlas. It was vvs- who tracked all the bugs and fixed lots of them. And now you tell us we're doing crap.

You really need to calm down and review your own ways to act here as a contributor.

I also see that project is very unorganized, and sometimes I want to run away and cry because I can't see such complicated and different code all around. But then I calm down and understand that no matter how bad it looks, yet it works.

Believe me, original TRs were even more messed up than OpenTomb. And this is part of our problem - we're dealing with very messed up "base material" here. I believe this is the reason why almost all TR open-source projects were abandoned at some point. Nobody figured out how to deal with activation masks, OCBs, packed bitfields, fixed-point numbers. Nobody replicated classic collision system. Nobody managed to learn the meaning of flipmaps, flipeffects, etc. We did. Maybe it's buggy, but we did it. We now even have reviewed 15-year-old document to comply with our discoveries.

If you want project to be feature-frozen, okay. Unfortunately, I'm not a pro graphics programmer, or physics specialist, or experienced game developer, so I just do what I can do. But I have lots of other interesting things to do in my life, and I'd better do them when you fix everything that we presumably broken or did wrong. You arrived here with a phrase implying that you want to maintain this project. Practically you do now, as TeslaRus is busy with other things.

I won't merge any pull requests, if you blame me for doing it without thought. Do it yourself.

vvs- commented 9 years ago

This project has already reached almost playable state. And I don't make self fulfilling prophecies.

How can you possibly enforce those rules? There is no payroll, no HR, no boss. Everyone comes and goes whenever he feels so. And why do you think that you have the authority to tell those people what they need to do? They made that code long before you and me helped them.

vvs- commented 9 years ago

Please, please. We all are not perfect. Don't say anything you'll feel sorry later. Just let your anger to subside before writing anything, ok?

Lwmte commented 9 years ago

I'm not angry. I can understand why Steffen says all that. But he must understand that now he's talking about things that weren't even in plan month ago. Ticket management? I didn't even knew what is it. Pull requests? What is this? Personally I thought that Git and GitHub is the same thing until recently.

I'm a newbie here. My only (seemingly) advantage among others here may be a better knowledge of some internal TR structures, as I spent ten years disassembling and patching original TR4 engine. If someone here is not happy with my coding habits, I can resign from coding and just give some directions. Someone with better knowledge and professional experience must manage the whole project.

The problem here, if you don't know what was previously done, most likely you will break it. It's exactly what happened month ago. But now it seems we're ones to blame.

stohrendorf commented 9 years ago

I just had to say this once, and probably won't say it again. I just had to get rid of it once, and now I feel better.

@Lwmte I honestly tried to fix the bugs. I spent day and night on debugging and research, and I'm still hunting bugs. I even had a very bad sleep for two weeks, because the code was so broken. The problem is that others are currently faster, so it looks like I'm not doing anything at all. And I'm not saying that everything is crap, I just wanted to suggest things that make things better. It sounded that bad because I had boiling emotions; reduce the rants to suggestions in that long post, please. And I don't want to be the project owner/leader/whatever, I just want some leader at all who makes a decision when things are discussed, because otherwise we would be discussing forever (see this ticket, or #115).

I see that this project is already fairly advanced, and this is great, but I also see some problems that from my experience probably make huge problems later if they aren't solved now or in the foreseeable future. I'd really like to support this project, and it also is fun working on it (most of the time).

And just one thing for a bit of clarification: Yes, I made that evil pull request #104, and I hacked everything in 6 days without properly testing it, but I also put a warning into the description that this should be tested and reviewed before merging, but it got merged about one hour after putting up that request. Now it's happened, and we don't need to discuss this further.

stohrendorf commented 9 years ago

@Lwmte If you got that much experience (wow!), then use it, comment on tickets with suggestions, review the code, etc. Write code, because only code tells the truth; comments and documentation are mostly inaccurate. And as a suggestion: if you're not sure about your coding, make it on a branch, and do a pull request, because then it can be discussed before it goes functional.

Lwmte commented 9 years ago

And I don't want to be the project owner/leader/whatever, I just want some leader at all who makes a decision when things are discussed, because otherwise we would be discussing forever

As I already said, I have no major experience in coding, neither in project maintenance. Probably that's why certain things had gone wrong. So let's choose someone who have proper experience. Or else we will be really discussing OpenGL context version for ages.

I made that evil pull request #104, and I hacked everything in 6 days without properly testing it

Maybe I'm misunderstanding the concept of pull request. I thought that pull request is done only when a person creating pull request thoroughly tested the code.

stohrendorf commented 9 years ago

Actually, pull requests are normally used to have a proper review, and let other test that change, too. If you test your own code, you probably don't catch all errors, because your mind "overwrites" the actual meaning of the code you're reading, which is a natural process. And if pull requests were only used for fully tested code, there wouldn't be a need for pull requests at all, as you could just commit to the master branch (not talking about people that don't have master access here).

At work, were planning to set up a local GitLab installation, just because of the pull request feature, which allows a great review process.

Gh0stBlade commented 9 years ago

I think we need to stick with 3.2 as much as I LOVE backwards compatibility. Some others who have experienced keeping it seem to say it's hassle and I believe it's only going to create more problems, glitches and possibly delays.

On the other hand, some of the posts here are exceptionally harsh and disappointed me. I just don't want to see the project fall apart like this and people get upset. Remember, we're a team of people who are trying our best to replicate the Tomb Raider engine. The smallest help and support has gotten it to where it is now.

I would like to say, originally the code was a complete mess. This is a fact, most variable names were inconsistent and furthermore, the lack of commenting of code/documentation can easily cause mis-interpretation of what something does and when edited, it could break anything.

Now, I certainly agree with some of the points stohrendorf has made. For example, proper reviews of pull requests. I missed the discussion of "every new feature on a branch", I do believe this will help keep the master as bug-free as possible. I don't recall many pull requests that caused severe bugs once merged though. Usually, I merge a pull request from my repo if it's a very small change and personally prefer to work on my own fork.

It seems that the lead developer Tesla Rus hasn't been active in a while. Maybe he's just busy but with all these changes, I hope he approves and isn't thrown off by the new style. The problem is that Tesla Rus has always been the lead and when he's absent very little gets done as I'm guessing the rest of us are mostly inexperienced enough to implement drastic features as from the looks of it, everyone has always waited for him.

I'm glad that more people have write access to the main repo, it has definitely benefited us, I remember when only Lwmte and Tesla Rus had that access which pretty much slowed down development. We've got some new people who have joined to help, it's greatly appreciated as we need as many contributors as we can possibly get.

At the current state of the project. I have to say, there has to be a time where the most severe issues are solved and nothing new is implemented as suggested (or all features are implemented on a branch until master is locked down and bug free). Right now, OpenTomb's biggest flaw is the collision system. I've not touched much of the collision code, I've tried to look at fixing some of these problems but it's out of my area. I don't know the core engine code much, I've refused to look due to it previously being too messy for me to handle. I honestly wish the collision system can be fixed because it's very frustrating how long it's been broken.

Finally, I'm happy with the code refactoring even though there are parts that I don't fully understand as I barely touch standard libraries (i hate them too). I will adjust and learn from the code no problem. I think we need to sort out some management, the most experienced programmers should take the lead. We need a better place to co-operate like a live IRC chat or something communication is something we need as a team which I've tried to suggest since joining.

Regards.

vvs- commented 9 years ago

I'm glad that discussion is now technical again. Let us keep it that way.

And I want to make some things clear. I never support any decision if it's:

That's for purely ideological reasons. I hope that people here understand the value of freedom and never sacrifice it for better technology.

cochrane commented 9 years ago

But I also thought about something else. Currently we have OpenTomb in pre-alpha stage, i. e. we still have no NPCs, no menus, no cutscenes support, collision is faulty... I think we need at least one or two years to make it stable. So anything may happen in these two years. First of all, market share of 3.2-capable hardware will be even larger than now. Second, maybe someone will write some kind of system to plug in various types of graphics APIs into it. Third, maybe Vulkan will be so cool and backwards-compatible that everyone will use it.

Vulkan won’t be backwards-compatible with OpenGL at all. It will be backwards compatible with itself (hopefully). I don’t know for sure, but I think I read something about it requiring 3.2 level hardware at least. In general, I would not worry about Vulkan right now. Let’s wait and see when it will be out and when the driver support is there. I might take a look at Apple’s Metal API in the meantime, to get used to such low-level APIs, but I would definitely vote against ever using that in OpenTomb.

As for angry: Oh yes, I've been that too. The fact that my entire build system is broken ( #115 ) and people insisted "that's how it should be" has not made me happy at all, and is the primary reason why I stopped working on this project lately. The thing is that it was probably the right decision to begin with, and with #247 it looks like we may finally have a solution (it works for me for bullet, and I'm currently trying to use it for other libraries as well). Me being angry contributed nothing to solving the issues.

I get the anger. But this isn't a professional project, frustrating as it may be. It would be great if it were, and I hope we can get there eventually, but we probably need to do more management work to figure out how.

stohrendorf commented 9 years ago

We simply need someone who makes decisions, and that specific person (only one) must be respected. As TeslaRus stated himself that he's not that experienced working in a team, I'd say he's out from being that person, but I'd prefer if he would be the one to choose one.

stohrendorf commented 9 years ago

Nothing about TeslraRus, but I think he's just not the one suited for making such decisions, although he would of course always have the last word ;)

stohrendorf commented 9 years ago

... and I just had the idea of setting up some non-public phpBB instance for doing such discussions, as some posts (especially mine) shouldn't have gone public in the first place, and tickets aren't supposed to be forum threads. But it's just an idea that would need further discussion.

vvs- commented 9 years ago

I'd prefer if he would be the one to choose one

This is not a monarchy :smile:

To get the same respect somebody should write code in this engine for years. The only one who has enough experience is @Lwmte. But of course he don't have all the experience of @TeslaRus. Still, he's the only one who provides any new functionality to the engine now.

stohrendorf commented 9 years ago

:+1: for @Lwmte, he seems to be the perfect CEO :smile:

vvs- commented 9 years ago

The thing is that it was probably the right decision to begin with, and with #247 it looks like we may finally have a solution (it works for me for bullet, and I'm currently trying to use it for other libraries as well). Me being angry contributed nothing to solving the issues.

The changes should not be disruptive. If something requires incompatible changes it should be done gradually and the impact should be estimated every time.

cochrane commented 9 years ago

The changes should not be disruptive. If something requires incompatible changes it should be done gradually and the impact should be estimated every time.

True. We should not have accepted that merge request as is back then (since it was never intended to be merged immediately). Especially not without any testing. This would have required way more discussion and probably splitting into many smaller items to merge. Let's mark this down as lessons learned for next time.

(And there will be a next time. There is still an awful lot of code that urgently needs to be refactored.)

Lwmte commented 9 years ago

There are actually more experienced people in my field - as you maybe seen in #38, @T4Larson has much more reverse-engineering experience, judging by his decompiled snippets. So you guys are a bit in a hurry making me a CEO! :smile:

@vvs- is pretty good at managing tickets, it'll be great if he can do this work in the future. I got basic idea of what should be done first, so I can prioritize them, but don't beat me if I do something wrong... Also I insist that someone else should at least manage pull requests, decide which feature should be integrated or not, etc. Just because I'm not a professional programmer, and I can't see advantage of this or that approach. Like, when somebody says "hey, let's use memory pools!", and I think yes, this is so cool and just like everybody does in gamedev. But then somebody says "we don't need memory pools, it's limited and complicated, and mostly used on consoles" - and I think for second time... Or our recent discussion with OpenGL version... I think - "so cool to have backwards-compatible engine", but then everybody say "2.1 is hell to make compatible with 3.2", and I re-think it...

So for this we need someone who is fluent enough in c++, as well as understand basic gamedev principles and trends in graphics.

vvs- commented 9 years ago

So you guys are a bit in a hurry making me a CEO!

No, because we don't need a professional programmer. We need somebody with a good experience in this particular engine.

but then everybody say "2.1 is hell to make compatible with 3.2", and I re-think it...

No, I don't think it's true. What everybody else thinks is not relevant, it's just the opinion of the people who actually will implement it what matters.

stohrendorf commented 8 years ago

I'm going to close this (bad memories, and outdated information).