search

WizzardMaker / AirlineTycoon

Source code of Airline Tycoon based on the GOG source code release
https://www.gog.com/game/airline_tycoon_deluxe
Other
26 stars 9 forks source link

feat: CLREWriter + Proper "modded.res" + Patched space station #52

Closed wackfx closed 4 months ago

wackfx commented 10 months ago

Hey there @WizzardMaker 👋

First off, so cool that you're maintaining this! It solved so many issues I had on Windows 11 (freeze on music change, crashes ...).

In order to get my feet wet to the project, I decided to improve french translation.

You'll find:

You can see this in actions in mods/misc/modded_res with the options you added.

Here is the translations in action image

⚠️ This would require to add the /misc/modded_ger.res to releases - but I believe it pave the way to more friendly modding and solves issues like #50

I didn't convert ALL translations - as I didn't know if you were interested in this or not. Let me know, I will gladly patch all the added text so far into this new system.

Cya!

PS: Small heads up - this PR is on top of your fix/various branch

wackfx commented 10 months ago

I've added some stuff:

image

wackfx commented 10 months ago

Actually converted (and translated) all texts from code into this new system.

wackfx commented 10 months ago

new stuff [fix] Patched graphical issue on route conf - some longer text would mix with price. Now patched image

[feat] Added an option to confire RoutePrice increment (asked on Discord)

image

[fix] Graphical issues with big numbers on BUY_KEROSENE screen - Now avoiding text to be scrambled with big numbers

image

wackfx commented 10 months ago

feat: Plane availability feature #45 Allows to add a new "availability" column to planetyp.csv in order to make the planes available on MUSEUM, BROKER or both. By default, or if the column doesn't exists (backcompatibility), the plane will be available in both. image

WizzardMaker commented 10 months ago

Happy to see so much progress!

I'll take a look at it in detail, as soon as I come back from my Christmas vacation, next year!

Your fixes to the text solution are very welcome! Having the strings in code was always a thorn in my eyes

Regarding your proposal to include modded files as separate entities - I already played with the idea of doing something similar. The only hurdle I saw at the time was the complexity of including those in a release on GitHub. There were already "complications" with instructing people to include the binary dependencies. For a similar project, I developed a companion application that downloaded/patched all necessary files and kept those up to date on a new version/release. The problem I see here though is the tech stack of this project. We are using C++, which is notorious for being difficult in working with HTTPS web requests across all the platforms, and GitHub only offers an HTTPS endpoint for their API.

But that's a story for a different PR - I think we can just bundle the additional files with the .exe, as they don't depend on any platform and are rather lightweight. Did you take a look at the "release" GitHub workflow, that bundles everything for releases? That would be the perfect place to zip all files up for release. I could also implement that, if you're not comfortable with workflows.

Any particular reason, why you chose to label those files as "mods" and not something like "patch". I see mods as more or less optional changes to a base, whereas patches are changes that shouldn't be optional - as are the changes you propose/added. Just a thought from my side, and nothing neck breaking for merging this.

The plan is to first merge my changes from "various-fixes" to develop, then check out your PR, merge that into develop and then merge develop finally into main.

Do you plan on adding anything else to this PR? It's getting rather large 😅

WizzardMaker commented 10 months ago

Could you also document your new features in the README.md (don't forget to credit yourself) when you are done?

WizzardMaker commented 10 months ago

I took a look at your new config entries - some of them are a bit troublesome for multiplayer, as these could lead to desync issues down the line on different settings. Especially because settings aren't synchronized across players.

This isn't an issue for now and probably only a few people are ever going to be changing these, but one should maybe add a system to check for a config mismatch. There should maybe be a split between gameplay/difficulty settings and client side settings, to allow for more granular/easy mismatch detection. But that's also an issue for later - if at all

wackfx commented 10 months ago

Hey @WizzardMaker, thank you for your comments!

About the text solution:

PR Size: You're very right, I might start creating new PR(s) on top of this one from now on to ease the review process 😅

Readme: No worries, will document everything!

Config: You're totally right, I'm not used to MP on this game. I believe a first fix would be to 1) use default values when in MP and only use those values for SP and maybe down the line 2) find a way to synchronise those parameters from the host maybe ?

WizzardMaker commented 10 months ago

Indeed, having a proper automatic patching system would be really great. I can take a look into it.

A possible solution would be a "launcher"/"version checker" that updates the game and all files with it.

I already implemented such a solution in C# for the Settlers 4 HD patch - but that application would need to be rewritten for a cross plattform UI, as currently only windows is supported. The underlying code however can be easily adapted (the base for that updater is: https://github.com/bezzad/Downloader) and I think it would suffice to maybe just have a command line version for now

WizzardMaker commented 10 months ago

Config: You're totally right, I'm not used to MP on this game. I believe a first fix would be to 1) use default values when in MP and only use those values for SP and maybe down the line 2) find a way to synchronise those parameters from the host maybe ?

I think it would be better to maybe add a notification to the multiplayer menu, when changes to those settings are detected. This makes multiplayer with those settings possible and notifies users that they should check their settings.

Synchronization is rather difficult due to the way how the settings class is implemented - that makes it rather cumbersome to change settings just for a single game.

wackfx commented 10 months ago

new stuff:

That would be the final missing piece for this (except with the automatic "update" system, which I believe would be better suited for another PR). Next commits would only be small stuff, like the multiplayer notice when config was changed + README new additions

WizzardMaker commented 10 months ago

Looks great! Love the unified workflow file, very clean.

Two things I noticed after a quick glance:

  1. The artifacts are archived twice, once in the pipeline and a second time by GitHub's download process image

  2. The artifacts contain the AT.json config file. This could lead to problems with inexperienced users that just copy/paste the archives contents into the game folder, thus overriding their configuration. The game should autogenerate with defaults by itself, when trying to access an undefined config.

wackfx commented 10 months ago

The artifacts are archived twice, once in the pipeline and a second time by GitHub's download process

Yeah, this unfortunately is a limitation brought by upload-artifact and download-artifact actions

Scenario 1 - we don't pack before upload-artifact

Scenario 2 - we pack before

I decided to go with the solution 2, as:

WDYT ?

The artifacts contain the AT.json config file. This could lead to problems with inexperienced users that just copy/paste the archives contents into the game folder, thus overriding their configuration. The game should autogenerate with defaults by itself, when trying to access an undefined config.

You're very correct about this - it's an overlook. I will patch this to avoid having the AT.json in archives

WizzardMaker commented 10 months ago

The important thing is, that the release files aren't zipped twice. Everything else is a nice to have. So, do as you see fit

wackfx commented 10 months ago

Ok, so, about the AT.json. It was generated because I'm using the executable as a CLI to generate the patched .res files with a command /update-patched-files-only that should transform .txt files to the final .res file and exit.

However, there was some class (like Sim) that was constructed (being global singleton) and that wrote AT.json, even if the game didn't start.

I've rewrote a bit Takeoff + Checkup and modified Sim to avoid various AT.json writing, we can thus use the CLI mechanism to actually execute part of C++ logic from our workflow, without creating artifacts files like this.

About the multiplayer notice for the settings: I can't make network game work on my computer anymore 😢 I tested with my build and your 1.7.1 Pre-Release (which did work previously when I tried) and no kudos. When I try to join with IP on Player 2 - it just doesn't do anything.

If you have any pointers about this issue, that'll be amazing. If not that's ok, I will try again later, maybe it's a random windows issue because my PC's been running for a couple of days now, will try a reboot tonight.

WizzardMaker commented 9 months ago

You need to rebase your branch on the new develop branch head

wackfx commented 8 months ago

Hey @WizzardMaker! I rebased my branch on top of the develop branch. No conflicts whatsoever as commits from your branch should be the same.

Also, I patched the MacOS build. I still have some issues with the MacOS json writer/reader but I believe this should be easy to fix