inexorgame-obsolete / deprecated-cube-engine-inexor

UNMAINTAINED: Please have a look at the vulkan-renderer
https://inexor.org
zlib License
11 stars 1 forks source link

Refactor the sourcebase more #550

Closed a-teammate closed 6 years ago

a-teammate commented 6 years ago

We might get close to finally removing game.hpp, engine.hpp and iengine.hpp and igame.hpp.

Maybe we can even finish it in this PR (its close at least)

a-teammate commented 6 years ago
 rename weapon.hpp to guns.hpp

Why? A chainsaw (and whatever we might have in the future) is not a gun.

They are named "GUNS" in the source code already. I just did what had to be done to get things splitted up. None of these names is set in stone, in fact I try to not think too much about single names as they are fucked anyways and I concentrate on getting the big structures right first. Means: First modularize the source file by file, afterwards split the individual files up and improve the modules in the same procedure.

a-teammate commented 6 years ago

If this runs through in the CI it can be merged too.

Croydon commented 6 years ago

None of these names is set in stone, in fact I try to not think too much about single names as they are fucked anyways and I concentrate on getting the big structures right first.

I agree when you keep things as they are. But you actively gave a file a worse name? 🤔

a-teammate commented 6 years ago

No, there still is a weapon.hpp (which exports the functions of the weapon.cpp) but there's also a guns.hpp for the data structure declarations. Btw weapon.hpp didn't exist before this branch :)

aschaeffer commented 6 years ago

Is this ready for a review?

a-teammate commented 6 years ago

Yes, the windows errors can be quickly fixed

Edit: they are fixed

a-teammate commented 6 years ago

Merge today eve

aschaeffer commented 6 years ago

Please wait until it works for me

a-teammate commented 6 years ago

Your setup is screwed, not the branch :D Change "libstd++" to "libstd++11" in your .conan/profiles/default

Croydon commented 6 years ago

Not neccessary. GCC5 is broken in the CI because of multiple errors in FMT/4.1.0 It might be the case that this only happens on Ubuntu Bionic for some reason, but we still need to fix this at some point.

Croydon commented 6 years ago

https://github.com/bincrafters/community/issues/101

a-teammate commented 6 years ago

fmt was working before, what changed? conan or fmt?

EDIT: It was compiling just fine for me with gcc 5.4

Croydon commented 6 years ago

fmt was working before, what changed? conan or fmt? EDIT: It was compiling just fine for me with gcc 5.4

I don't know yet what caused this. It could be broken only in some newer Ubuntu version. On what OS + version did you try it @a-teammate ?

a-teammate commented 6 years ago

It is definitely compiling for me with gcc 5.4 on OS Mint 18.

I made a small revert in so far as I removed a commit which was solely adding the glm dependency (without using it)

@aschaeffer: 1) make sure in your conan settings (~/.conan/profiles/default) is libcxx libstdc++11 and not libstdc++ 2) remove all conan packages again

aschaeffer commented 6 years ago

Will try tonight :-)

Croydon commented 6 years ago

So I updated spdlog in my branch and the fmt error is now gone. But gRPC 1.1.0 is broken too with GCC5/Ubuntu Bionic. I'm almost certain now that this has something to do with the update to Ubunto Bionic. (Which might be fixed in newer gRPC versions, which is still blocked by #553).

However, it's fine for me to go on with this, when it works for Hanack's GCC5 setup.


None of these names is set in stone, in fact I try to not think too much about single names

Git is really bad in keeping track of file name changes, so file name changes should be kept to a minium required. Tracking back changes beyond the point of a single rename is already hard enough. So maybe it was named "guns" in the source, but weapons would have been still been a better fitting file name even for the current code as it contains a chainsaw. It's much easier to change names within a file at some later point.


Good work in general @a-teammate, but maybe consider several smaller pull requests next time.

a-teammate commented 6 years ago

We found the error of @aschaeffer's setup. It is a generic error on ubuntu or any other distro which has mir installed (which is only ubuntu).

When SDL2 builds with mir support, we have two protobuf inside inexor. One from mir, which is leaking symbols.
mir ships with its own an very old protobuf. And hence we got issues on startup, since "we" use two different versions of protobuf in the same app.

We were able to fix it, by fixing the SDL2 recipe. Is there any place where our currently used SDL2 recipe is uploaded? (the fix is simple self.run('cd %s && ./configure --enable-video-mir=no' % (self.folder)))

EDIT: Oh, it is my personal SDL2 repo, I see. EDIT2: Uhm and it even already contained that fix. Why is this package corrupt if it actually should be fixed already?

Croydon commented 6 years ago

That's because it's only in your master branch, while we are using your stable/2.0.5 release branch (https://github.com/lasote/conan-sdl2/pull/15)

a-teammate commented 6 years ago

It is also in the PR..