TTimo / GtkRadiant

The open source, cross platform level editor for idtech games
http://icculus.org/gtkradiant/
Other
582 stars 152 forks source link

Merge NetRadiant's Q3Map2 updates into GtkRadiant one #294

Open Mateos81 opened 8 years ago

Mateos81 commented 8 years ago

Lately, Q3Map2 had some changes in the NetRadiant repo, for instance: https://gitlab.com/xonotic/netradiant/commit/c2d78b967469afcf2a640209f23f7345feabd3c7 https://gitlab.com/xonotic/netradiant/commit/c6378c2c623e72d1890feda4afc85d25cb636d9b

Might it be a good idea to merge these updates into the compiler supplied with GtkRadiant?

Edit: Extra link of current work on Q3Map2 for NetRadiant: https://gitlab.com/illwieckz/netradiant/activity

TTimo commented 8 years ago

Sure, not opposed to it..

Mateos81 commented 8 years ago

What do you think about having a common repository for Q3Map2, since it is the common compiler for all id Tech III based games, and thus all Radiant forks? That would avoid people making changes on their side and the need to merge left and right these changes; Something unified would maybe strengthen its dev, perhaps clean a bit its code base and optimize it?

TTimo commented 8 years ago

I like that the q3map2 source that is in the GtkRadiant project right now is reasonably stable and trustworthy. The last round of fixes that was done to it was by Rambetter, and he carefully worked some test cases to validate his fixes.

Getting some other q3map2 source from a project I haven't followed exposes us to a lineage of code changes that we have no control over. Bugs introduced in q3map2 are hard to find, and hard to fix. Side effects are hard to control there, and there is very little knowledge around of how this tool works.

I don't know that we need some third repository anyway. That's not really a git way of doing things.

Maybe we could compile and bundle netradiant's q3map2 and offer it as an option when configuring a game?

Mateos81 commented 8 years ago

Good points, sounds good to me :)

TTimo commented 8 years ago

That other version of q3map2 likely won't have the TCP/XML feedback reporting? Other concern is to have 2015 project files for it. I suppose when configuring a game we could prompt which version to use.

TTimo commented 8 years ago

If there is a good git url to pull from to obtain a source tree for this version of q3map2 that will compile standalone, I'd like to get it as part of the regular GtkRadiant build.

I think during setup we would need to prompt the user which version of q3map he wants to use or something to that order.

TTimo commented 8 years ago

@illwieckz any input on this? I'm thinking I'll drop this from the 1.6.5 milestone otherwise.

illwieckz commented 8 years ago

Oh, I missed this issue. It's a good idea to merge some stuff like the fix linked in the first comment and the help mechanism. Merging fixes or things unrelated to the build itself (like the interactive help) is safe. Also, adding functionalities that were never in GtkRadiant source tree is not going to break existing functionalities (for example adding the minimap generation will not break the bsp compilation).

I can work on some merge yes. By the way, I'm very busy these days, @TTimo do you have a date planned for the 1.6.5 milestone?

I think it's not a high priority to merge functionalities that are not in GtkRadiant's q3map2 : if people need more, they already know it's provided by another binary. The high priorities are fixes for bugs. Also, the help will be very convenient for many. I can work on that.

About the idea to ship multiple q3ma2, it's a good idea. It can allow prudent people to use well-known and predictable stuff (even if it means relying on bugs :wink: ) while allowing tweakers to hack new stuff. I see there is already two q3map2 in GtkRadiant source tree: the vanilla one and the UrT one. It could be very good to be able to get multiple q3map2 version available and define a way to let gamepacks and users to prefer a q3map2 version.

TTimo commented 8 years ago

This makes a lot of sense. If there are easy, low hanging fruit things I'd like to land them asap. Everything else can wait, we would probably move this work to the next milestone to keep an eye on it.

illwieckz commented 7 years ago

For information this is a tool I use to easily diff various radiant/q3map2 trees: compare_radiant.

The tool fetches trees and translates them to a common common layout then apply a common uncrustify profile on them so it's easy to diff them. This is the tool I use to diff q3map2 from GtkRadiant and q3map2 from NetRadiant of course. This way it's easy to find differences and if some makes sense, I just have to look for the related commit in the related history.

From what I've read, q3map2 from NetRadiant is not very far from the one from GtkRadiant, it's probably one of the less modified one compared to some other q3map2 forks that do nothing more but were completely revamped. Most of the changes only add functionalities, and some, minor, add fixes.

So I think it can be safe to merge some of these functionalities 1 by 1, it's something I'm trying to do: identify changes and making unitary PR for these. Hopefully, GtkRadiant and NetRadiant still shares the same history, so it's easy to add GtKradiant as a NetRadiant remote and vice versa to cherry-pick needed commits, taking care of titles, authors, dates etc.

It would be very good to narrow a lot the q3map2 diff between NetRadiant and GtkRadiant trees. The NetRadiant one is basically the same with more options and more functionnalities than GtkRadiant one. Except fixes for bugs, the common things have not changed, it means a merge is doable.

TTimo commented 7 years ago

Pulling this out of the 1.6.5 milestone. We did get a number of fixes from compiler warnings and such, I don't have a precise plan to continue from there. Some of the bugs that would potentially be worth addressing in q3map2 include issue #390 for instance.