Add OpenBSD support. #5706

ibara · 2017-06-25T19:39:55Z

Hello --

This patch allows OpenRCT2 to successfully build and run on OpenBSD.
I'll briefly walk through the patch:

src/openrct2-ui/UiContext.Linux.cpp: Add a defined(OpenBSD) at the top. Additionally, in IsSteamOverlayAttached(), since there is no Steam on OpenBSD, simply return false. Perhaps this can be reframed to be #ifdef __linux__ ... code ... #else return false; #endif so other platforms can benefit but I will leave that to you to decide.

src/openrct2/Version.h: Add OpenBSD OPENRCT2_PLATFORM.

src/openrct2/core/FileStream.cpp: The BSDs are like Mac OS X and Android in regards to fseeko and ftello functions (which makes sense, since Mac OS X is based on FreeBSD and Android libc is OpenBSD libc).

src/openrct2/core/String.hpp: Need #include <stdarg.h> to get va_args, at least on OpenBSD.

src/openrct2/localisation/localisation.c: The iconv charset name is UTF-8, not UTF8, at least on OpenBSD.

src/openrct2/platform/linux.c: Add defined(OpenBSD). Also, we know the exePath for the openrct2 binary will always be /usr/local/bin/ on OpenBSD so don't bother doing anything fancy for discovering exePath.

I intend to add OpenRCT2 to OpenBSD's official package repository once this is committed, and I will make sure that OpenRCT2 continues to build and run on OpenBSD.

Thanks!

janisozaur · 2017-06-25T19:46:18Z

src/openrct2/core/String.hpp

@@ -16,6 +16,7 @@

 #pragma once

+#include <stdarg.h>


Since this is a C++ file, can you rather use cstdarg here instead?

janisozaur · 2017-06-25T19:48:09Z

src/openrct2/platform/linux.c

@@ -56,6 +56,8 @@ void platform_get_exe_path(utf8 *outPath, size_t outSize)
        log_fatal("failed to get process path");

    }
+#elif defined(__OpenBSD__)
+    strlcpy(exePath, "/usr/local/bin/", sizeof(exePath));


How can you be sure it will always be /usr/local/bin? What about the case when user compiles it somewhere in his workspace? Even the FreeBSD part just above interrogates this information from the system dynamically.

Because we only care about our package system. If a user compiles it elsewhere, then it's up to them to edit this line.
OpenBSD doesn't have a sysctl like FreeBSD to get the path anyway (at least, not easily, and as far as we're concerned, without benefit).

Would a fallback mechanism be possible? Something along the lines "check if there is /usr/local/bin/openrct2 executable" and perhaps "am I the /usr/local/bin/openrct2" and if these return false, warn user about it?

Yes, can do something like get dirname of executable and check against it. Should it just warn, or should it error?

Just warn should be fine.

janisozaur · 2017-06-25T19:49:49Z

@ibara that's lovely, thanks!

I've left some line notes, can you please respond to them?

We also have few tests (you may need to pass -DWITH_TESTS=on to cmake, but it may be the default), can you run them (make all && make test) and post if they all run and pass?

Gymnasiast · 2017-06-25T19:51:15Z

About the iconv name: doing iconv -l on Linux lists both UTF-8 and UTF8.

ibara · 2017-06-25T19:52:32Z

@Gymnasiast ours only lists UTF-8. We're using libiconv-1.14, perhaps there's a difference between that and the new 1.15?

@janisozaur Sure, I will amend my patch and run the tests.

Gymnasiast · 2017-06-25T19:55:53Z

@ibara My iconv is at version 2.24, but it's the glibc version. That's probably the cause. But since it supports either name, it's no problem to change it to UTF-8.

janisozaur · 2017-06-25T20:00:25Z

@ibara https://github.com/OpenRCT2/OpenRCT2/blob/develop/distribution/changelog.txt#L90

You can use [ci skip] anywhere in the commit message to not trigger CI runs ;)

Gymnasiast · 2017-06-25T20:02:18Z

I noticed that you added defines for OpenBSD in places where there is no corresponding FreeBSD define. Have you got any idea why this is? Are there simply some ifdefs missing for FreeBSD?

ibara · 2017-06-25T20:07:28Z

@Gymnasiast Unfortunately, unlike the Linuxes, each BSD is an entirely separate operating system. It is possible that FreeBSD does things differently from OpenBSD. My guess is that FreeBSD should be added where I have added OpenBSD, but to be certain you would need to check with one of their developers.

I can say, however, that defined(FREEBSD) looks wrong to me: it should likely be defined(FreeBSD).

janisozaur · 2017-06-25T20:09:26Z

It was based on https://github.com/OpenRCT2/OpenRCT2/pull/4916/files, however that PR happened when we were still using SDL2's defines.

Gymnasiast · 2017-06-25T20:11:16Z

@ibara Hm, I see. Probably best to let someone who knows more about FreeBSD look at it, then. It might not even compile any more.

This is also why I'm very happy you said this:

I intend to add OpenRCT2 to OpenBSD's official package repository once this is committed, and I will make sure that OpenRCT2 continues to build and run on OpenBSD.

ibara · 2017-06-25T20:13:19Z

@Gymnasiast I can only speak for myself (as an OpenBSD developer) but we make a point of staying active with our upstreams when we add a package to our package repo.

Gymnasiast · 2017-06-25T20:13:49Z

We'll also need an entry in the changelog, and in some other documents as well, like readme.md.

ibara · 2017-06-25T20:15:17Z

OK. Let me finish building this current diff with WITH_TESTS=on and then I'll write something up for those.

ibara · 2017-06-25T20:46:28Z

@janisozaur Tests finished.
Tests 1-4 passed.
Tests 5 and 6 failed.

Let me know what you need for me to investigate.

Additionally, I did some asking around, it appears there is in fact no way to get the process path name of a running executable in OpenBSD :(
So what's in src/openrct2/platform/linux.c is probably the best effort that can be made. I can leave a note somewhere for someone not using the OpenBSD package to make sure to change that line.

janisozaur · 2017-06-25T20:52:47Z

@ibara can you run the failing tests manually (they're regular binaries of name test_xxx in your build dir) and provide their output? I'm guessing they are the ride_ratings_test and multilaunch_test, both of which require having set up RCT2 assets and OpenRCT2 data correctly, which I can only guess is because you didn't really launch it from /usr/local/bin as you fake that information.

Can you try changing the hardcoded path to one that you actually used for compilation and retry?

I suppose the comment will have to suffice, if there's no way to figure out the real path.

ibara · 2017-06-25T20:57:28Z

My fault, All the tests pass. I moved the data directory before running them. Put the data directory back in the right place, and they all passed. :)
All is good.

ibara · 2017-06-25T21:03:46Z

My CMake skills aren't the best. If there's a better way to do what I did in test/tests/CMakeLists.txt, please let me know.

janisozaur · 2017-06-25T21:04:52Z

In that case: add entry to the changelog, add comment about user having to set the path manually and I'm happy to approve this.

Other things you could also verify if they work:

user's locale is autodetected (language, temperature and distance units, currency)
user's username is autodetected (when opening multiplayer window, there is an editbox on top of it)
multiplayer server list gets populated (ensures curl's certificates are all used properly)
connecting to any of the supported live multiplayer servers works (tests OpenSSL)

Can you also comment on what kind of packages you have in OpenBSD? pre-built or portage-style?

ibara · 2017-06-25T21:08:21Z

@janisozaur It's best to think of OpenBSD like apt-get. While OpenBSD is technically portage-style (as portage simply stole the BSD ports system which already exited for 10 years before portage started), OpenBSD makes pre-built packages, and those packages are the only things we officially support.

janisozaur · 2017-06-25T21:10:27Z

Then also please update this section: https://github.com/OpenRCT2/OpenRCT2#2-downloading-the-game-pre-built (might be in future PR, once you have the package done)

ibara · 2017-06-25T21:18:08Z

Blah, sorry I will eventually learn how CI works...
Please double-check everything. I will submit another PR once packages are being built.

janisozaur · 2017-06-25T21:18:27Z

@ibara can I amend your commits a little bit?

ibara · 2017-06-25T21:18:46Z

Please do.

ibara · 2017-06-25T21:26:35Z

Tested these:
user's locale is autodetected (language, temperature and distance units, currency)
user's username is autodetected (when opening multiplayer window, there is an editbox on top of it)
multiplayer server list gets populated (ensures curl's certificates are all used properly)
connecting to any of the supported live multiplayer servers works (tests OpenSSL)

Everything checks out except autodetecting username. I will look into it.

janisozaur · 2017-06-25T21:30:22Z

I'm done with amending the commits, please update your local clone.

janisozaur · 2017-06-25T21:33:56Z

Everything checks out except autodetecting username. I will look into it.

it is possible that is some recent regression, I have seen some empty-named users in multiplayer recently, see #5707

ibara · 2017-06-25T21:41:05Z

We should be good.

rwjuk · 2017-06-25T21:45:06Z

I'm just setting up an OpenBSD install (a new OS for me!) to test with, once I've successfully built & tested the game this should be good to go.

ibara · 2017-06-25T21:50:27Z

Please make sure you install -current and not the latest release (grab what you need from here: http://ftp4.usa.openbsd.org/pub/OpenBSD/snapshots/amd64/ to ensure it's -current).

rwjuk · 2017-06-25T21:51:50Z

@ibara Noted, thanks. What could break if users aren't on -current out of interest? Or is it just best practice to always run the latest version of everything (as in Arch, etc.)

ibara · 2017-06-25T21:56:20Z

All development is done on -current, and we only support packages from the moment they are imported into the tree (which is always done on -current). So the first OpenBSD release that will have an OpenRCT2 package will be OpenBSD 6.2. However, it will always be supported on -current.
In practice, the vast majority of our desktop users use -current, partly because that's how you get new things :)

rwjuk · 2017-06-25T22:03:57Z

@ibara Ahh, that makes sense. Thanks - always good to learn something new :)

ibara · 2017-06-25T22:24:25Z

@rwjuk You will also need some extra patches and setup (not things to be upstreamed), see here: https://github.com/jasperla/openbsd-wip/tree/master/games/openrct2

Or (perhaps the best method) I can show you how to build OpenRCT2 through our ports system (which is how packages get made).

rwjuk · 2017-06-25T22:29:05Z

@ibara Oh, so I can't just grab the source and compile? That's what I'm trying to do at the moment, but I'm getting cc1: error: unrecognised command line option "-std=gnu11". I imagine that's due to the old version of gcc OpenBSD seems to come with (4.2.1).

EDIT: Ahh, seems from your setup that I need to use clang. Okay, can do.

ibara · 2017-06-25T22:31:31Z

There is clang-4.0.0 in the base install, but it's not yet the default compiler.
However, you have no other dependencies installed (like SDL2, or cmake, or ...).

rwjuk · 2017-06-25T22:32:53Z

@ibara Oh I grabbed those okay via pkg_add. What do I need to do with the stuff you linked to do it the 'proper' way? (Sorry, BSD newbie!)

ibara · 2017-06-25T22:35:03Z

It's fine.

There's a couple of patches here: https://github.com/jasperla/openbsd-wip/tree/master/games/openrct2/patches

Then tell cmake your compiler is clang and pass -DCMAKE_SHARED_LINKER_FLAGS="-L/usr/X11R6/lib" to cmake (I think this is a cmake bug) and you should be good to go.

rwjuk · 2017-06-25T22:36:48Z

@ibara I'm assuming those patches aren't to be included because the bugs they work around are being fixed upstream? And thanks!

ibara · 2017-06-25T22:37:36Z

One of them I need to talk to the cmake maintainers about, the other one is we're missing a macro in assert.h (and we need to fix that!)

rwjuk · 2017-06-26T14:34:48Z

Just FYI @ibara and others who might be interested, I was able to build & run OpenRCT2 on OpenBSD successfully! 👍

- Feature: [#1399 (partial), #5177] Add window that displays any missing/corrupt objects when loading a park - Feature: [#5056] Add cheat to own all land. - Feature: [#5133] Add option to display guest expenditure (as seen in RCTC). - Feature: [#5196] Add cheat to disable ride ageing. - Feature: [#5504] Group vehicles into ride groups - Feature: [#5576] Add a persistent 'display real names of guests' setting. - Feature: [#5611] Add support for Android - Feature: [#5706] Add support for OpenBSD - Feature: OpenRCT2 now starts up on the display it was last shown on. - Feature: Park entrance fee can now be set to amounts up to £200. - Improved: Construction rights can now be placed on park entrances. - Improved: Mouse can now be dragged to select scenery when saving track designs - Fix: [#259] Money making glitch involving swamps (original bug) - Fix: [#441] Construction rights over entrance path erased (original bug) - Fix: [#578] Ride ghosts show up in ride list during construction (original bug) - Fix: [#597] 'Finish 5 roller coasters' goal not properly checked (original bug) - Fix: [#667] Incorrect banner limit calculation (original bug) - Fix: [#739] Crocodile Ride (Log Flume) never allows more than five boats (original bug) - Fix: [#837] Can't move windows on title screen to where the toolbar would be (original bug) - Fix: [#1705] Time Twister's Medieval entrance has incorrect scrolling (original bug) - Fix: [#3178, #5456] Paths with non-ASCII characters not handled properly on macOS. - Fix: [#3346] Crash when extra long train breaks down at the back - Fix: [#3479] Building in pause mode creates too many floating numbers, crashing the game - Fix: [#3565] Multiplayer server crash - Fix: [#3681] Steel Twister rollercoaster always shows all track designs - Fix: [#3846, #5749] Crash when testing coaster with a diagonal lift in block brake mode - Fix: [#4054] Sorting rides by track type: Misleading research messages - Fix: [#4055] Sort rides by track type: Sorting rule is not really clear (inconsistent?) - Fix: [#4512] Invisible map edge tiles corrupted - Fix: [#5009] Ride rating calculations can overflow - Fix: [#5253] RCT1 park value conversion factor too high - Fix: [#5400] New Ride window does not focus properly on newly invented ride. - Fix: [#5489] Sprite index crash for car view on car ride. - Fix: [#5730] Unable to uncheck 'No money' in the Scenario Editor. - Fix: [#5750] Game freezes when ride queue linked list is corrupted. - Fix: [#5819] Vertical multi-dimension coaster tunnels drawn incorrectly - Fix: Non-invented vehicles can be used via track designs in select-by-track-type mode. - Fix: Track components added by OpenRCT2 are now usable in older scenarios. - Technical: [#5047] Add ride ratings tests - Technical: [#5458] Begin offering headless build with reduced compile- and run-time dependencies - Technical: [#5755] Title sequence wait periods use milliseconds - Technical: Fix many desync sources

janisozaur reviewed Jun 25, 2017

View reviewed changes

Brian Callahan added 3 commits June 25, 2017 23:25

Add OpenBSD support.

fe8f949

Use cstdarg instead of stdarg.h

e5d305f

Add a note for OpenBSD and getting path names of running executables.

46ae057

Brian Callahan added 2 commits June 25, 2017 23:27

Add needed documentation.

0dac1f2

No -ldl on the BSDs, so make sure only Linux gets it

e7da549

rwjuk mentioned this pull request Jun 25, 2017

Usernames may not be automatically detected for multiplayer #5707

Closed

janisozaur approved these changes Jun 25, 2017

View reviewed changes

Gymnasiast merged commit ce16693 into OpenRCT2:develop Jun 26, 2017

Add OpenBSD support. #5706

Add OpenBSD support. #5706

Conversation

ibara commented Jun 25, 2017

janisozaur Jun 25, 2017

Choose a reason for hiding this comment

ibara Jun 25, 2017

Choose a reason for hiding this comment

janisozaur Jun 25, 2017

Choose a reason for hiding this comment

ibara Jun 25, 2017

Choose a reason for hiding this comment

janisozaur Jun 25, 2017

Choose a reason for hiding this comment

ibara Jun 25, 2017

Choose a reason for hiding this comment

janisozaur Jun 25, 2017

Choose a reason for hiding this comment

janisozaur commented Jun 25, 2017

Gymnasiast commented Jun 25, 2017 • edited Loading

ibara commented Jun 25, 2017

Gymnasiast commented Jun 25, 2017

janisozaur commented Jun 25, 2017

Gymnasiast commented Jun 25, 2017

ibara commented Jun 25, 2017

janisozaur commented Jun 25, 2017

Gymnasiast commented Jun 25, 2017

ibara commented Jun 25, 2017

Gymnasiast commented Jun 25, 2017

ibara commented Jun 25, 2017

ibara commented Jun 25, 2017

janisozaur commented Jun 25, 2017

ibara commented Jun 25, 2017

ibara commented Jun 25, 2017

janisozaur commented Jun 25, 2017 • edited Loading

ibara commented Jun 25, 2017

janisozaur commented Jun 25, 2017

ibara commented Jun 25, 2017

janisozaur commented Jun 25, 2017

ibara commented Jun 25, 2017

ibara commented Jun 25, 2017

janisozaur commented Jun 25, 2017

janisozaur commented Jun 25, 2017

ibara commented Jun 25, 2017

rwjuk commented Jun 25, 2017

ibara commented Jun 25, 2017

rwjuk commented Jun 25, 2017

ibara commented Jun 25, 2017

rwjuk commented Jun 25, 2017

ibara commented Jun 25, 2017

rwjuk commented Jun 25, 2017 • edited Loading

ibara commented Jun 25, 2017

rwjuk commented Jun 25, 2017

ibara commented Jun 25, 2017

rwjuk commented Jun 25, 2017

ibara commented Jun 25, 2017

rwjuk commented Jun 26, 2017

Gymnasiast commented Jun 25, 2017 •

edited

Loading

janisozaur commented Jun 25, 2017 •

edited

Loading

rwjuk commented Jun 25, 2017 •

edited

Loading