search

mickael9 / ioq3

This is an up-to-date fork of ioquake3 for UrbanTerror with changes from the ioUrbanTerror engine
GNU General Public License v2.0
42 stars 18 forks source link

Added discord integration #27

Closed danielepantaleone closed 6 years ago

danielepantaleone commented 6 years ago

This pull request introduce Discord RPC integration. The engine now exposes new API calls so that the game module can interact with the Discord RPC library to fill rich presence data.

Also some new VM calls have been defined for the game module to react to Discord specific events:

danielepantaleone commented 6 years ago

I just noticed that Travis failed the build:

ld: warning: ignoring file libs/macosx/libdiscord-rpc.dylib, file was built for x86_64 which is not the architecture being linked (i386): libs/macosx/libdiscord-rpc.dylib

I think i386 build on macOS can be safely disabled since the last 32bit macOS version is Leopard (from 2009). From that release many more happened.

If we want to support older macOS versions we would need to build the discord library ourselves on a 32bit macOS VM, since I don't think Discord developers are going to provide 32bit macOS builds. But I think it's not worth at all.

danielepantaleone commented 6 years ago

Ay update on this? Is this something you are considering to integrate in the engine?

KroniK907 commented 6 years ago

Is there a way that you can have this just build successfully without the discord code on i386 builds on MacOS rather than having this merge cause any build for that to fail?

danielepantaleone commented 6 years ago

No. As of today, Discord is distributed only as 64 bit lib. We would need to recompile it ourselves but it’s not worth it. MacOS on 32 bit is something that I do not see since 10 years or so.

KroniK907 commented 6 years ago

Wait, Now I'm confused. Is there no way to tell it to not include or build the discord code for the i386 MacOS build? Shouldn't it just be a matter of writing a case in the makefile to disable all discord code for that particular build? (This is coming from someone who has very little experience in C or C++ and their build processes).

danielepantaleone commented 6 years ago

Sorry I misread. Yes that can be done. I thought you wanted Discord on 32 bit macOS.

KroniK907 commented 6 years ago

I think that would be better than merging code that would exclude a platform that was previously supported from any further updates. Sure there may only be like 2 people in the world that still want to run UrT on i386 MacOS, but I would bet money that someone would end up on the forums complaining about it if support was dropped for that platform (Since we are aiming to try and get FS to use this build as the official build in the future).

ThomasBrierley commented 6 years ago

Sure there may only be like 2 people in the world that still want to run UrT on i386 MacOS, but I would bet money that someone would end up on the forums complaining about it if support was dropped for that platform

I think it should be safe to drop 32bit mac, I did a little 64bit OS X patch for the official engine almost two years ago and i386 was dropped in the process due to the added complexity of binary stitching and dealing with multiple copies of the mac SDL binary etc (I don't think this was even mentioned in the release notes):

https://github.com/FrozenSand/ioq3-for-UrbanTerror-4/pull/52

I was actually a bit apprehensive about this because I don't like obsoleting hardware unnecessarily, but there has been no complaints that I know of, and as Daniel mentioned these 32bit machines are well over a decade old now and there aren't that many of them (I believe only a few models just after the PPC switch), if people are still using them they are probably using OSS anyway.

I'm actually using a 10 year old MBP and I had to switch to Linux because it's way to old for Apple's behemoth - I'm pretty sure i'm unusual in that respect but even mine is 64bit.

KroniK907 commented 6 years ago

Well in that case, it seems this could be pulled as is, and have the i386 MacOS build removed from the travis, since UrT hasn't supported it since 2016.

danielepantaleone commented 6 years ago

I will then exclude Discord integration from makefile on macOS i386 architecture (for consistency) and remove the i386 macOS build from travis.....when I find some free time 😃

ThomasBrierley commented 6 years ago

I just noticed there are still linux and windows i386 discord binaries included... this makes sense since i386 OS X is long deprecated but the other OS are not. So this patch doesn't actually obsolete 32bit mac hardware with discord, just the OS (which would be a bad idea for anyone to use anyway since it's been abandoned by Apple).

ThomasBrierley commented 6 years ago

What about other platforms though? i.e BSDs. The USE_DISCORD option could be ANDed with another SUPPORTED_DISCORD option that is explicitly set by each supported platform/arch combo (5). The inverse could be done, explicitly setting USE_DISCORD to 0 for every unsupported combo, but that seems a bit verbose seeing as there are many more that will not support it.

That way unsupported platforms automatically disable it and build cleanly without configuration.

danielepantaleone commented 6 years ago

I'm closing this one. Please see: https://github.com/mickael9/ioq3/tree/discord