DaemonEngine / Daemon

The Dæmon game engine. With some bits of ioq3 and XreaL.
https://unvanquished.net
BSD 3-Clause "New" or "Revised" License
306 stars 60 forks source link

Nuking the obsolete deprecated legacy forward renderer? #747

Open illwieckz opened 2 years ago

illwieckz commented 2 years ago

I think I'm going to nuke the obsolete dynamic lighting renderer. It doesn't work anymore on master (no dynamic light rendered) and the only reason why it is kept is that it has some features not ported to the tiled renderer.

But those features are either broken (dynamic shadows) or unused so maybe broken (no one knows how to use them).

For now I kept them around because it was an interesting reference implementation someone could borrow interesting things when re-implementing those features on the tiled renderer.

The legacy forward renderer is considered as deprecated since 2 years: https://github.com/DaemonEngine/Daemon/commit/be7229a2489971e14a3a636d962117012449478a

But no one tried to port the features in those two years, and unless there is an hidden skill in some of us right now no one of the active people has the skill to do such port.

Nuking this code will nuke thousands of lines of code.

What's your opinion?

DolceTriade commented 2 years ago

I was wondering why dyanmic lighting looked so weird when I've been playing to the point I'd rather have it be disabled all together :(

I wish I could use the old forward renderer :smile:

necessarily-equal commented 2 years ago

I don't really have an opinion on this since

  1. I have never seen it working;
  2. I don't know that part of the code so I don't know how useful or useless this can be as a reference implementation to be ported;
  3. I don't know how much of a maintenance pain it can be.

I think if you want to nuke it it probably means it's not worth keeping it around as you try hard to keep legacy code.

In fact I thought the legacy renderer was already dropped.

illwieckz commented 1 year ago

To remove confusions:

In fact I thought the legacy renderer was already dropped.

There was two things:

Those were even two separate code trees, two separate libraries. We dropped the first one a decade ago.

Then, in the remaining Modern OpenGL 3 renderer, there is the part that is used to render lights and things like that, it's more a subset of the whole “Modern OpenGL 3 renderer”, it's a set of functions and a different code path.

So let's say we have two code paths in the “Modern OpenGL 3 renderer” and we may NUKE one of them.

VReaperV commented 1 month ago

This can be done once the required features from #1209 are implemented.