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

[Bug]: Install target for emscripten is incorrect #9655

Closed
kaomoneus opened this issue Oct 25, 2021 · 2 comments · Fixed by #11934
Closed

[Bug]: Install target for emscripten is incorrect #9655

kaomoneus opened this issue Oct 25, 2021 · 2 comments · Fixed by #11934

Comments

@kaomoneus
Copy link

Version of OpenTTD

12.0-RC1

Expected result

As an installaction result I expect to see 4 files gather in same directory:

  • openttd.html
  • openttd.data
  • openttd.js
  • openttd.wasm

Actual result

Only openttd.html appears in final installation directory (install/games dir).

Also share directory is created as if would install it on Linux.

Steps to reproduce

  1. Initialize build directory with cmake. Use custom installation path by means of -DCMAKE_INSTALL_PREFIX=<>
  2. Build openttd.
  3. Install it.
  4. Observe installation directory.
@kaomoneus
Copy link
Author

I believe we need something like this:
kaomoneus@ef03a56

My concerns (about my fix)

  1. .js, .wasm and .data perhaps should be classified as a data files. But there is no such component. So I just heaped everything in Runtime.
  2. I hardcoded paths like ${CMAKE_BINARY_DIR}/openttd.js but perhaps, there is some variable I'm not aware of. Smth like OPENTTD_JS_PATH
  3. Perhaps it's good to redefine default CMAKE_INSTALL_DATADIR and CMAKE_INSTALL_BINDIR values. For emscripten it is better just to keep structure flat, all in same directory.

@TrueBrain TrueBrain mentioned this issue Jan 30, 2024
Merged
@TrueBrain
Copy link
Member

You should have made a PR out of it! It went under the radar now :D

  1. that is fine.
  2. that is fine, and pretty common.
  3. nah; this is fine :)

I did make a change to your commit, as not installing COPYING etc isn't right :) But other than that, tnx for the commit!

TrueBrain added a commit that referenced this issue Jan 30, 2024
@TrueBrain
Fix #9655: [Emscripten] install the correct files on install (#11934)
2321517 
Co-authored-by: Stepan Dyatkovskiy (kaomoneus) <ml@dyatkovskiy.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix #9655: [Emscripten] install the correct files on install TrueBrain/OpenTTD
2 participants
@kaomoneus @TrueBrain