triplea-game / triplea

TripleA is a turn based strategy game and board game engine, similar to Axis & Allies or Risk.
https://triplea-game.org/
GNU General Public License v3.0
1.34k stars 393 forks source link

File Association Missing on Linux #5994

Closed DanVanAtta closed 4 years ago

DanVanAtta commented 4 years ago

How can the problem be recreated?

When the problem is fixed, what should have happened instead?

Need to decide what a fix would look like: A) we support this on all OS B) we remove the supporting code for this C) we make the supporting code OS specific and only executed when on a specific OS

There is a broader point we should decide as well if we really should support features that only work on some OS but not for everyone. For example, the Mac variant of this feature says "sorry, we do not support this, yet still tries to do so). We also need to be really sure the ROI of this feature is worth our effort and ongoing manual test (which for OS specific code is very costly).

Which Engine Version are you using? 2.0.8171 (or below)

Screenshots

Zipped Save Game

DanVanAtta commented 4 years ago

Another element to consider for the broader discussion of if we support OS specific features, any such features will not work for Debian and should not even be compiled into Debian. This adds work on any release for the Debian people to remove those features, or for us to do that for them. This makes it less likely for TripleA to be available at all as a Debian packaged distribution.

RoiEXLab commented 4 years ago

I created #5996 which should fix the file-association Problem on Linux systems that follow the freedesktop.org specification.

Another element to consider for the broader discussion of if we support OS specific features

Difficult to answer. For us as developers simply dropping those kind of requirements would certainly make things easier, but at the same time some people just "expect" certain things on their OS. This includes where the "About" screen can be found on mac os, using Cmd instead of Ctrl there (which I noticed when I tried zooming using Ctrl on a mac recently) as well as opening tsvg files. Other features like URL handling are a nice bonus feature, so people can look at maps on https://triplea-game.org/maps-list/maps/ and directly download them from there. In theory those features shouldn't be a big deal, if we ignore the fact that I implemented URL handlers "manually" by hacking install4j features together so the installer would register TripleA accordingly which I replaced with the install4j mechanism in #5960 then we just have to implement code that looks at the first commandline argument and calls code accordingly. Easily reproducable. The only problem we're really having is apples operating system that does a lot of things differently, which only makes things harder because no one here does own a private mac for testing purposes. That's the reason why we have to use javas desktop API because applications on mac are somewhat "singleton", and apple likes to send custom events to the application rather than re-invoking the actual executable like it happens on other operating systems.

In the end I think we should keep "OS-specific" things, because they just feel right most of the time. There is room for improvement though, most notably the "if isMacOs" statements in the code have a lot of room for improvement: By hiding those kind of system dependent oddities behind some kind of abstraction (using interfaces or just helper methods that hide the complexity) we do not only simplify the code by a considerable amount, it would also help decoupling the code from the actual UI implementation

DanVanAtta commented 4 years ago

Any thoughts to the debian packaging? All of the os specific code is removed in patches as well as non compliant dependencies. We want debian releases to be a pure downstream fork.

Many games do not load save files, just launch the game. I think if something is not uniform across OS, it could get weird. It tends to start feeling like the browser wars, one adds a feature, another a different, then suddenly development needs to multiply efforts to keep it all working.

I also think java is partly here not to have such OS customization, "write once, run anywhere".

On Tue, Mar 3, 2020, 4:51 AM RoiEX notifications@github.com wrote:

I created #5996 https://github.com/triplea-game/triplea/pull/5996 which should fix the file-association Problem on Linux systems that follow the freedesktop.org specification.

Another element to consider for the broader discussion of if we support OS specific features

Difficult to answer. For us as developers simply dropping those kind of requirements would certainly make things easier, but at the same time some people just "expect" certain things on their OS. This includes where the "About" screen can be found on mac os, using Cmd instead of Ctrl there (which I noticed when I tried zooming using Ctrl on a mac recently) as well as opening tsvg files. Other features like URL handling are a nice bonus feature, so people can look at maps on https://triplea-game.org/maps-list/maps/ and directly download them from there. In theory those features shouldn't be a big deal, if we ignore the fact that I implemented URL handlers "manually" by hacking install4j features together so the installer would register TripleA accordingly which I replaced with the install4j mechanism in #5960 https://github.com/triplea-game/triplea/pull/5960 then we just have to implement code that looks at the first commandline argument and calls code accordingly. Easily reproducable. The only problem we're really having is apples operating system that does a lot of things differently, which only makes things harder because no one here does own a private mac for testing purposes. That's the reason why we have to use javas desktop API because applications on mac are somewhat "singleton", and apple likes to send custom events to the application rather than re-invoking the actual executable like it happens on other operating systems.

In the end I think we should keep "OS-specific" things, because they just feel right most of the time. There is room for improvement though, most notably the "if isMacOs" statements in the code have a lot of room for improvement: By hiding those kind of system dependent oddities behind some kind of abstraction (using interfaces or just helper methods that hide the complexity) we do not only simplify the code by a considerable amount, it would also help decoupling the code from the actual UI implementation

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/triplea-game/triplea/issues/5994?email_source=notifications&email_token=AC6SZONPIOQU7R3HRBBXJ7LRFT4LVA5CNFSM4LAFMRHKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOENTLTEY#issuecomment-593934739, or unsubscribe https://github.com/notifications/unsubscribe-auth/AC6SZOIUQGCIHPJG7GRIDM3RFT4LVANCNFSM4LAFMRHA .

RoiEXLab commented 4 years ago

Any thoughts to the debian packaging? All of the os specific code is removed in patches as well as non compliant dependencies. We want debian releases to be a pure downstream fork.

I'm not entirely sure what the process is there, but from what I observed it seems like the people over at debian seem to "convert" all of our maven dependencies into normal package dependencies so that the triplea package can actually depend on it rather than shipping as a fat jar. So there's already this kind of work that's happening there. Previously our eawt dependency was causing issues, because those kind of classes were only present on JDK <9 versions on macOS in all other cases the dependency was no-op. However starting with Java 9 this sort of mechanism became API as part of java.desktop. Some time ago I dug into the actual source code and on systems that don't support a particular "desktop API" like passing file opening events to a running process the corresponding implementation is just NO-OP. So if they're depending on the "normal" OpenJDK for triplea to run, then this "normal" JDK should already be able to deal with those kind of requirements, no manual work involved there. However if their goal is to keep the package as small as possible by "optimizing" redundant code on linux systems then this would be IMO unecessary work.

In the end it would be really helpful to know the procedure for creating the triplea package: I have no idea what actually needs to be done for it to work thus I have a hard time arguing over the potential implications I don't fully understand, but as of now I don't see any problems with making it a debian package

DanVanAtta commented 4 years ago

Not all of our deps are compatible with the debian license requirements. The json package we were using, mp3 package, and iirc something to support apple as well were all rewritten to not need those packages.

To the larger point though, anything that is not straight up minimal, or can't be readily tested with a single OS, imo we finally need to properly value our time and call those features as not worth it.

On Tue, Mar 3, 2020, 4:10 PM RoiEX notifications@github.com wrote:

Any thoughts to the debian packaging? All of the os specific code is removed in patches as well as non compliant dependencies. We want debian releases to be a pure downstream fork.

I'm not entirely sure what the process is there, but from what I observed it seems like the people over at debian seem to "convert" all of our maven dependencies into normal package dependencies so that the triplea package can actually depend on it rather than shipping as a fat jar. So there's already this kind of work that's happening there. Previously our eawt dependency was causing issues, because those kind of classes were only present on JDK <9 versions on macOS in all other cases the dependency was no-op. However starting with Java 9 this sort of mechanism became API as part of java.desktop. Some time ago I dug into the actual source code and on systems that don't support a particular "desktop API" like passing file opening events to a running process the corresponding implementation is just NO-OP. So if they're depending on the "normal" OpenJDK for triplea to run, then this "normal" JDK should already be able to deal with those kind of requirements, no manual work involved there. However if their goal is to keep the package as small as possible by "optimizing" redundant code on linux systems then this would be IMO unecessary work.

In the end it would be really helpful to know the procedure for creating the triplea package: I have no idea what actually needs to be done for it to work thus I have a hard time arguing over the potential implications I don't fully understand, but as of now I don't see any problems with making it a debian package

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/triplea-game/triplea/issues/5994?email_source=notifications&email_token=AC6SZOIXWAOMFSLNXEPLPKDRFWL6BA5CNFSM4LAFMRHKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOENVVLDI#issuecomment-594236813, or unsubscribe https://github.com/notifications/unsubscribe-auth/AC6SZOMMHFJYNAEOWJI724TRFWL6BANCNFSM4LAFMRHA .

DanVanAtta commented 4 years ago

For OS specific co-OP, imo it's not easy to maintain. The code is there and looks like it should do something. That the behavior is actually os specific is subtle and not readily tested. Any such intrinsic code I think should be wrapped in an os check so it's explicit behavior, otherwise it seems too clever/nuanced.

On Tue, Mar 3, 2020, 5:32 PM Dan Van Atta dhvatta@gmail.com wrote:

Not all of our deps are compatible with the debian license requirements. The json package we were using, mp3 package, and iirc something to support apple as well were all rewritten to not need those packages.

To the larger point though, anything that is not straight up minimal, or can't be readily tested with a single OS, imo we finally need to properly value our time and call those features as not worth it.

On Tue, Mar 3, 2020, 4:10 PM RoiEX notifications@github.com wrote:

Any thoughts to the debian packaging? All of the os specific code is removed in patches as well as non compliant dependencies. We want debian releases to be a pure downstream fork.

I'm not entirely sure what the process is there, but from what I observed it seems like the people over at debian seem to "convert" all of our maven dependencies into normal package dependencies so that the triplea package can actually depend on it rather than shipping as a fat jar. So there's already this kind of work that's happening there. Previously our eawt dependency was causing issues, because those kind of classes were only present on JDK <9 versions on macOS in all other cases the dependency was no-op. However starting with Java 9 this sort of mechanism became API as part of java.desktop. Some time ago I dug into the actual source code and on systems that don't support a particular "desktop API" like passing file opening events to a running process the corresponding implementation is just NO-OP. So if they're depending on the "normal" OpenJDK for triplea to run, then this "normal" JDK should already be able to deal with those kind of requirements, no manual work involved there. However if their goal is to keep the package as small as possible by "optimizing" redundant code on linux systems then this would be IMO unecessary work.

In the end it would be really helpful to know the procedure for creating the triplea package: I have no idea what actually needs to be done for it to work thus I have a hard time arguing over the potential implications I don't fully understand, but as of now I don't see any problems with making it a debian package

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/triplea-game/triplea/issues/5994?email_source=notifications&email_token=AC6SZOIXWAOMFSLNXEPLPKDRFWL6BA5CNFSM4LAFMRHKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOENVVLDI#issuecomment-594236813, or unsubscribe https://github.com/notifications/unsubscribe-auth/AC6SZOMMHFJYNAEOWJI724TRFWL6BANCNFSM4LAFMRHA .