Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Make game runnable on posix #14

Merged
merged 18 commits into from Jan 24, 2018
Merged

Make game runnable on posix #14

merged 18 commits into from Jan 24, 2018

Conversation

marijnvdwerf
Copy link
Member

No description provided.

IntelOrca
IntelOrca reviewed Jan 20, 2018
View reviewed changes
src/openloco/platform/platform.posix.cpp Outdated
@@ -12,6 +12,7 @@

int main(int argc, const char * * argv)
{
openloco::lpCmdLine((char *)argv[0]);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lpCmdLine is actually the full command line given to the application. You would need to join argv together with space separated, quotes if necessary. Or just leave it unset and implement the parser wherever it is.

@AaronVanGeffen AaronVanGeffen mentioned this pull request Jan 21, 2018
Closed
@marijnvdwerf marijnvdwerf force-pushed the posix-build-2 branch 3 times, most recently from fce030f to 0f000a1 Compare January 21, 2018 14:05
@AaronVanGeffen
Copy link
Member

FWIW, in its current state, I can get the PR to compile on Arch Linux, but running it immediately leads to a segfault:

(gdb) run
Starting program: /home/aaron/Repositories/OpenLoco/build/openloco 
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/usr/lib/libthread_db.so.1".

Program received signal SIGSEGV, Segmentation fault.
0x0116eaed in openloco::interop::loco_global<char*, 5395272u>::operator=(char*) ()

@marijnvdwerf
Copy link
Member Author

Seems to be an issue with loco_global<char *, 0x00525348> glpCmdLine;

@janisozaur
Copy link
Contributor

Required me to change casing of files from what GOG provides, but it works just fine.

Here's what the filenames should be:

20s1.DAT
20s2.DAT
20s3.DAT
20s4.DAT
20s5.DAT
20s6.DAT
40s1.DAT
40s2.DAT
40s3.DAT
50s1.DAT
50s2.DAT
50s3.DAT
60s1.DAT
60s2.DAT
60s3.DAT
70s1.DAT
70s2.DAT
70s3.DAT
80s1.DAT
80s2.DAT
80s3.DAT
80s4.DAT
90s1.DAT
90s2.DAT
ch.dat
Chrysanthemum.DAT
CSS1.DAT
CSS2.DAT
CSS3.DAT
CSS4.DAT
CSS5.DAT
Eugenia.DAT
g1.DAT
g1k.dat
g1tc.dat
GAME.CFG
hang.dat
KANJI.DAT
PLUGIN2.DAT
PLUGIN.DAT
Rag1.DAT
Rag2.DAT
rag3.DAT
Scores.DAT
tai.dat
title.dat
TUT1024_1.DAT
TUT1024_2.DAT
TUT1024_3.DAT
TUT800_1.DAT
TUT800_2.DAT
TUT800_3.DAT
tutk1024_1.dat
tutk1024_2.dat
tutk1024_3.dat
tutk800_1.dat
tutk800_2.dat
tutk800_3.dat

@AaronVanGeffen
Copy link
Member

Confirmed working with the data files matching case. These really ought to be case-insensitive, but for now… 👍

@Gymnasiast
Copy link
Contributor

Gymnasiast commented Jan 22, 2018 •
edited

When I try to build this branch, I get:
/home/michael/Programma's/Code/CLionProjects/OpenLoco/src/openloco/interop/hook.cpp:195:72: error: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 2 has type ‘uintptr_t {aka unsigned int}’ [-Werror=format=]

@janisozaur
Copy link
Contributor

@Gymnasiast Ubuntu's GCC has -Wformat enabled as default, it is also seen on Travis. You're welcome to fix it or temporarily disable this error on your end.

@Gymnasiast
Copy link
Contributor

@janisozaur I believe that Marijn changed this in this branch, this line was fine before.

@janisozaur
Copy link
Contributor

clang suggests %x

@janisozaur
Copy link
Contributor

janisozaur commented Jan 22, 2018 •
edited

But ideally you should be using PRIuPTR from <inttypes.h> and it's C++ counterpart: http://www.cplusplus.com/reference/cinttypes/

@janisozaur janisozaur force-pushed the posix-build-2 branch 2 times, most recently from 134839a to 3e8ff06 Compare January 23, 2018 21:49
@marijnvdwerf marijnvdwerf force-pushed the posix-build-2 branch 3 times, most recently from e82f920 to 328db4b Compare January 23, 2018 22:49
marijnvdwerf
marijnvdwerf commented Jan 23, 2018
View reviewed changes
src/openloco/interop/hooks.cpp
write_jmp(0x4078be, (void*)&fn_4078be);
write_jmp(0x4078f8, (void*)&fn_timeGetTime);
// write_jmp(0x4078f8, (void*)&fn_timeGetTime);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably remove these alltogether?

IntelOrca
IntelOrca reviewed Jan 23, 2018
View reviewed changes
src/openloco/interop/hook.cpp Outdated
static void* _smallHooks;
static uint8_t* _offset;

static void* makeJump(uint32_t address, void* fn)
Copy link
Collaborator

@IntelOrca IntelOrca Jan 23, 2018 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make_jump

How does this differ from write_jmp, should it just replace it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This pushes the address onto the stack, before calling the function. It's used to hook all the library functions. The offset is then pointed to the new operations.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we always use this instead of write_jmp?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so. It would need to generate different assembly based on whether cdecl or stdcall is used. write_jmp currently is used for both types. This is only used for 'If lib function is used that we haven't tackled, print the address to the command line'.

IntelOrca
IntelOrca reviewed Jan 23, 2018
View reviewed changes
src/openloco/interop/hooks.cpp Outdated
@@ -1,11 +1,16 @@
#include <assert.h>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#include <cassert>

@janisozaur janisozaur mentioned this pull request Jan 24, 2018
Merged
janisozaur
janisozaur reviewed Jan 24, 2018
View reviewed changes
src/openloco/interop/interop.cpp
@@ -357,7 +361,7 @@ namespace openloco::interop
if (left != right)
{
uint32_t addr = lhs.begin + i;
std::printf("0x%06X: %02hhX %02hhX\n", addr, std::to_integer<unsigned char>(left), std::to_integer<unsigned char>(right));
std::printf("0x%06X: %02X %02X\n", addr, (uint8_t)left, (uint8_t)right);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh well…

@janisozaur janisozaur merged commit f25e9f4 into master Jan 24, 2018
@janisozaur janisozaur deleted the posix-build-2 branch January 24, 2018 14:05
marijnvdwerf added a commit to marijnvdwerf/OpenLoco that referenced this pull request Feb 21, 2019
@marijnvdwerf
Merge pull request OpenLoco#14 from marijnvdwerf/19-01/shortcuts
33da272 
Keyboard shortcut handling
@TransshipmentEnvoy TransshipmentEnvoy mentioned this pull request Apr 12, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@janisozaur janisozaur janisozaur left review comments

@IntelOrca IntelOrca IntelOrca approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@marijnvdwerf @AaronVanGeffen @janisozaur @Gymnasiast @IntelOrca