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.

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

13197 - Mac interface cleanup and Legacy preference #107

Closed
wants to merge 0 commits into from

Conversation

Cattlesquat
Copy link
Collaborator

@Cattlesquat Cattlesquat commented Jul 19, 2020

Stage 1 - This should clean up the rest of the Mac Mouse bullshit and add the legacy preference. Keyboard shortcuts will be more of a bear.

Rather than have methods named e.g. "isRightMouse" but they lie to us and screw with our heads when we deal with Macs (wait, does "Control" mean "Control" right now, or does "Control" mean "Command"?), I have created names that hopefully are both understandable when read by a non-initiate but also "different enough" to remind us they have special meaning (and let us keep things straight in our heads when dealing with Macs).
SwingUtils.isVanillaLeftButtonDown() -- is left mouse button down ("except on Mac when it's pretending to be right mouse")
SwingUtils.isContextMouseButtonDown() -- is right button down (OR on Mac is left button pretending to be right button)
SwingUtils.isSelectionToggle() -- normally "Ctrl+LeftClick" on non-Macs. Other things on Macs, depending on "stuff".
SwingUtils.isShortcutKeyDown() -- ... in development. Name may slightly evolve if my needs do. Normally "Ctrl". On Mac usually "Command" but when legacy pref "Control"

This was the easy part, partially because of all the work done on this for 3.3.2.

Beating the keyboard shortcuts into line is going to be more "uncharted territory" so a good thing for me to work on ... tomorrow!

@yanlyub
Copy link
Contributor

yanlyub commented Jul 19, 2020

This is good.

There's a lot of missing Override annotations in that area, perhaps you could add them, they make navigating the class/interface structure easier for humans and development tools:

interface AnInterface {
  void methodA();
  void methodB();
}

class AClass implements AnInterface {
  @Override
  void methodA() {}

  @Override
  void methodB() {}
}

@Cattlesquat
Copy link
Collaborator Author

Will do

@Cattlesquat
Copy link
Collaborator Author

STAGE 2 - Module KeyStroke's translated back and forth from "Command" to "Control" at the appropriate places.
SwingUtils.getKeySystemToVassal() - Translate's Mac's platform-specific Command key into "Ctrl" for storing-in-module purposes
SwingUtils.getKeyVassalToSystem() - Translate's Vassal-stored keystrokes from "Ctrl" into Command key if we're on a Mac for checking-for-actual-keystroke purposes.

  • if macLegacy preference is in effect, then this is all just a pass-through.

Here is a secondary detail that I need advice on: I need to do ONE of the following:
(a) We could make macLegacy a "requires restart" preference. In this case I'd need @yanlyub's advice on how to fix my little configurer path so that SwingUtils.setMacLegacy() does get called at Vassal startup, so that SwingUtils knows what mode we're in, but that it does not get called every time the user checks/unchecks the preference (but locally GlobalOptions.setPrefsMacLegacy() still gets called, or in some way the preference gets properly written out)
(b) OR, someone could suggest a mechanism/algorithm to have every VASSAL.tools.KeyStrokeListener know to "unregister" and then re-"register" its keystrokes.

@Cattlesquat
Copy link
Collaborator Author

Cattlesquat commented Jul 19, 2020

Meanwhile, @uckelman we are probably ready for some test builds so that I can test this on Mac. The thought of installing a dev environment on my wife's Macbook Pro sends cold chills up my spine...

Unfortunately there will probably of course be a few rounds of test builds, Unless God Has Granted A Miracle.

@Cattlesquat
Copy link
Collaborator Author

Or if there's some way that "through the magic of maven" I can cause a mac build to get made here on my Windows machine...

@yanlyub
Copy link
Contributor

yanlyub commented Jul 19, 2020

It is still a "make" project on Linux, maven builds the Java part but the make system takes care of packaging the final distribution.

If you can install git and have a Java installation, you could clone the repo, use the maven wrapper to build the jar, unzip the Vassal's "other" distribution somewhere, replace it's jar files with the ones you built, and start it using the VASSAL.sh script.

@Cattlesquat
Copy link
Collaborator Author

I'd have to figure out how to do all of that on a Mac (sort of like having my right arm tied behind my back and several fingers of my left hand removed), and during the brief windows I can borrow my wife's laptop!

Now if there's a way to build a Mac version on my Windows machine...

@Cattlesquat
Copy link
Collaborator Author

Cattlesquat commented Jul 19, 2020

@yanlyub Investigating installing a Ubuntu virtual machine on my Windows. We'll see how far I get :)

(By "investigating", I mean the install "has 19 minutes remaining...")

@Cattlesquat
Copy link
Collaborator Author

@yanlyub Okay, so let's say I've got a little Ubuntu virtual machine running, and it has "git" and "java 11 sdk" installed for at least some value of "installed". Like I can type "java" at a $ prompt and get a spew of java-y stuff. And I can type "git" and I've told it I exist (user.name and stuff) and it recognizes me.

I "shared" my Windows folder of vassal over, but when I type "git status" in that folder from the Ubuntu side of the world, I get an immense spew of "everything in the world has changed", so in other words, git isn't "really" playing well with that folder (on Windows side typing "git status" in same folder says I'm all clean).

So maybe I'd better make a new folder in my Ubuntu drive and just pull Vassal down into THAT.

But meanwhile, what OTHER stuff do I need to get to be able to build the makefile? I tried "sudo apt-get install make" and it "made something install", but when I typed make in the vassal directory it was "sad".

@uckelman
Copy link
Contributor

Changes to everything is due likely to line ending differences between Windows and Linux.

What you need for making Mac builds is genisoimage (for which there will be an Ubuntu package) and dmg (for which there is probably not an Ubuntu package). I was already intending to see about updating dmg and providing a simple way to build it; I may run that down in the coming week.

@Cattlesquat
Copy link
Collaborator Author

I've gotten "surprisingly far". What does one type to tell the makefile to build a Mac version (as opposed to just compiling the build)? Also... is "dmg" the name of a specific program? I thought it was more of a file format?
image

@yanlyub
Copy link
Contributor

yanlyub commented Jul 19, 2020

Yes git on linux handles line endings differently from windows, just clone the repo into a new folder inside your VM.

You could install several more JDKs, they come in handy when you want to try different Java and Vassal versions, Vassal 3.2 will need JDK 8. You could install these three for instance: a JDK 8 for running Vassal 3.2, JDK 11 as it's the long-term release and the minimum required JDK for building Vassal 3.3, and JDK 14 as it's the most modern one and it's VM should run Vassal in the most performant way.

For the makefile to build a release, ultimately you will have to ask @uckelman as I never tried setting up my machine to do the full build cycle, I only know that you can install nsis and genisoimage from the package manager.

To install all these packages:

sudo apt install openjdk-14 openjdk-11 openjdk-8 nsis genisoimage

You can also type sudo apt install openj or sudo apt install geniso then keep hitting tab, apt has tab-completion nowadays.

One of the Javas will be set as the default, you can choose which one like this: https://askubuntu.com/a/740782

But even then you can still choose which one you use by using the absolute path, e.g.:

$ /usr/lib/jvm/java-11-openjdk-amd64/bin/java -version
$ /usr/lib/jvm/java-8-openjdk-amd64/bin/java -version

(They all are in /usr/lib/jvm/<jdkversion>/bin/java and you can use tab completion after typing /usr/lib/jvm/)

In Eclipse / IntelliJ you can add any available JDK in the compiler settings (IntelliJ will automatically find them anyways) and select which one is used for the project.

@yanlyub
Copy link
Contributor

yanlyub commented Jul 19, 2020

I've gotten "surprisingly far". What does one type to tell the makefile to build a Mac version (as opposed to just compiling the build)?

It's make release-macosx, and you see all available make targets in the last line of the Makefile, behind the .PHONY:. And make has tab completion too, you can just type make then hit tab a couple times, or for instance make r + tab + tab + -m + tab + tab.

@Cattlesquat
Copy link
Collaborator Author

Here's how far I get on "make release-macosx" so far.
image

@uckelman
Copy link
Contributor

Here's how far I get on "make release-macosx" so far.
image

Huh, so wget didn't create the jdks directory for you?

@Cattlesquat
Copy link
Collaborator Author

Well THAT was easily remedied with mkdir.
Next up:
image

@uckelman
Copy link
Contributor

Are you current with master? I suspect you're getting a stray newline in tmp/module_deps, and there's a commit on master to fix that.

@Cattlesquat
Copy link
Collaborator Author

Seems to think everything is up to date

@Cattlesquat
Copy link
Collaborator Author

I'd pulled the version down for the very first time a couple hours ago.

@uckelman
Copy link
Contributor

It looks like tmp/module_deps wasn't written out correctly, then. Whats in it?

@Cattlesquat
Copy link
Collaborator Author

jdk.crypto.ec,Error: unknown option: --ignore-missing-depsUsage: jdeps <path...>]use --help for a list of possible options

lol

@Cattlesquat
Copy link
Collaborator Author

Maybe it doesn't like my Java 11 jdk. Still trying to figure out where to get 14. Yan's command earlier didn't find it.

@yanlyub
Copy link
Contributor

yanlyub commented Jul 20, 2020

Which ubuntu version did you install?

@Cattlesquat
Copy link
Collaborator Author

how do I tell?

@Cattlesquat
Copy link
Collaborator Author

By the way I did manage to get a jdk14 installed, and got a lot farther. Am now missing, as Joel predicted, libdmg-hfsplus

@Cattlesquat
Copy link
Collaborator Author

Apparently I am about to go on a "Voyage of Discovery" of downloading THAT repository and compiling it :)

@yanlyub
Copy link
Contributor

yanlyub commented Jul 20, 2020

Try sudo apt install build-essential first, this will install the whole C compiler toolset. Then you might need to install single libraries like libcrypto in their development versions, you can search the ubuntu repository with apt search libcrypto, and the package you need usually has the lib- prefix and -dev suffix e.g. libssl-dev, install that one with sudo apt install.

@Cattlesquat
Copy link
Collaborator Author

Here is where currently stuck (this is in trying to build libdmg-hfsplus)
image

@Cattlesquat
Copy link
Collaborator Author

I have installed all of the things you described above. Of course I'm probably missing something else. I googled an article about how to install/build libdmg-hfsplus and followed all of those steps, but it doesn't remedy this.

@yanlyub
Copy link
Contributor

yanlyub commented Jul 20, 2020

I just tried it myself, getting the same compile error in filevault.c or filevault.h or however this build log is supposed to be read.

@uckelman
Copy link
Contributor

A thing I just remembered: You don't actually need dmg for building a test DMG. dmg is what compresses the DMG output by genisoimage. We want a compressed DMG for releases, but if all you're doing is testing, you can comment that out an use the uncompressed one.

@uckelman
Copy link
Contributor

Yes I can't get libssl1.0-dev for my ubuntu 20.04 either, they don't keep all the old versions in their repos. Ugh. Unmaintained C/C++ projects and their dependency management. Care to find out how to manually build the old libssl 1.0 lib? rofl

This is all unnecessary if you guys just give me a chance to deal with dmg. There's a much newer version than the one I'm using. I probably can't compile the one I'm using now either---I compiled it a long time ago and never bothered to update it because I was the only one using it and it works.

@yanlyub
Copy link
Contributor

yanlyub commented Jul 20, 2020

An automated build would be great, we could give it to any CI server and you wouldn't be the only guy on this planet anymore who can produce all the necessary releases.

@Cattlesquat
Copy link
Collaborator Author

@uckelman Totally great to deal with dmg but in the meantime I actually got it to compile (got the old libssl through some hanky panky from one of those articles). So you're absolutely right that's probably not the thread we want everything to hang from but I was "on a tear" and got it running. Need a Mac build? ;)

@yanlyub Oh you're probably right it's probably still got old-style modifier in the mask and then adding the new one back in because it sees that. Whee!

@uckelman
Copy link
Contributor

If you're asking Java to do something with key modifiers, there's a good shot that if it sees one of the *_MASK_DOWN modifiers set that it will also set the corresponding *_MASK modifier, and vice versa, for backwards compatibility. You might be seeing that. (There's a SequenceEncoder test which looks for exactly that thing.)

@Cattlesquat
Copy link
Collaborator Author

Yep that was it. This should go quickly now :-)

@Cattlesquat
Copy link
Collaborator Author

Cattlesquat commented Jul 20, 2020

Okay, next little mystery, @uckelman and @yanlyub. I'm less mystified by "why this doesn't work now" and more mystified by "how did this ever work". The following is from ForwardToKeyBuffer. Note that we're sending all three of "keyPressed", "keyTyped", and "keyReleased" in to "process". And there's special code explicitly designed to consume & ignore the "keyTyped", so yay for that. But then the keyReleased arrives and gets processed, and there's nothing here designed to consume/ignore it.

So the behavior I'm seeing is... my "translated" command gets translated correctly, and it gets recognized correctly... and it activates TWICE! And yet this is the exact logic that was in here before, apart from me changing the getKeyStrokeForEvent() to filter through my special method.

Any ideas? (Also code markup refuses to indent this code, for no reason I can find)

`
@OverRide
public void keyPressed(KeyEvent e) {
process(e);
}

@OverRide
public void keyReleased(KeyEvent e) {
process(e);
}

@OverRide
public void keyTyped(KeyEvent e) {
process(e);
}

protected void process(KeyEvent e) {
// If we've consumed a KeyPressed event,
// then automatically consume any following KeyTyped event
// resulting from the same keypress
// This prevents echoing characters to the Chat area if they're keycommand for selected pieces
if (lastConsumedEvent != null
&& lastConsumedEvent.getWhen() == e.getWhen()) {
e.consume();
}
else {
lastConsumedEvent = null;
}
final int c = e.getKeyCode();
// Don't pass modifier keys alone to counters
if (!e.isConsumed() && c != KeyEvent.VK_SHIFT && c != KeyEvent.VK_CONTROL && c != KeyEvent.VK_ALT && c != KeyEvent.VK_META) {
Command comm = KeyBuffer.getBuffer().keyCommand
(SwingUtils.getKeyStrokeForEvent(e));
if (comm != null && !comm.isNull()) {
GameModule.getGameModule().sendAndLog(comm);
e.consume();
lastConsumedEvent = e;
}
}
}
`

@yanlyub
Copy link
Contributor

yanlyub commented Jul 20, 2020

I can only approach this from the theoretical perspective, having almost no practical experience with these keyboard/mouse parts of Java.

Isn't "keyTyped" same as "keyPressed" and "keyReleased" combined into one event? And if "keyPressed" is processed and the "keyTyped" that belongs to that same "keyPressed" is ignored, shouldn't the "keyReleased" that also belongs to the "keyPressed" be ignored as well?

@Cattlesquat
Copy link
Collaborator Author

Well that's the logic I'd be coming at it with too, and yet "somehow this always worked". So I'm wondering if there's "secret Java voodoo" at work (like the kind where the KeyStroke regrew its CTRL_DOWN_MASK because of the deprecated flag still being on)

@Cattlesquat
Copy link
Collaborator Author

Okay I think I've found it. There's an extra piece of KeyStroke I need to make sure I put into the translated version, because we're apparently using it.

@Cattlesquat
Copy link
Collaborator Author

(And yes, after I got my first Mac build done last night, I realized - DUH - that I could debug 95% of this by just making a 'Windows' version of the Input Classifier to swap some keys)

@Cattlesquat Cattlesquat marked this pull request as ready for review July 20, 2020 19:15
@Cattlesquat
Copy link
Collaborator Author

DONE! And works on wife's actual Mac. Will deserve a beta, but that's what 3.3.3 is planned to have anyway.

@Cattlesquat
Copy link
Collaborator Author

@uckelman @yanlyub This is my "Merge Candidate 0" for this Mac Panacea Patch, so go ahead and review when you have a chance.

@Cattlesquat
Copy link
Collaborator Author

Meanwhile, I think I've created a monster...
image

@yanlyub
Copy link
Contributor

yanlyub commented Jul 20, 2020

Looks good, I left a few minor suggestions, and if you could change the base class for the unit tests VASSAL.tools.swing.AbstractSwingUtilsTestIsMouseButton in lines 46 and 61, make it test the new methods instead of the deprecated ones, then I won't have to do that later on 😁

@uckelman
Copy link
Contributor

@Cattlesquat Have a look at PR #109. It has bootstrap.sh, which you can run to download and the JDKs needed for packaging, and it also downloads and compiles a current version of dmg.

@Cattlesquat
Copy link
Collaborator Author

This latest attempt to merge appears to have "deleted the entire project" in this branch. Need to figure out how to rewind.

@Cattlesquat
Copy link
Collaborator Author

@yanlyub uh.... I most definitely did not mean to "close this". I was trying to get rid of the part where somehow merging deleted the whole project. Any hints?

@uckelman
Copy link
Contributor

Hmm. Let me mess with this, as I was hoping to look at this next.

@Cattlesquat
Copy link
Collaborator Author

Yeah it all seemed fine until I tried to merge 13196 into it after you merged that.

@Cattlesquat
Copy link
Collaborator Author

And then I tried to rewind it but apparently not the right way.

@Cattlesquat
Copy link
Collaborator Author

I still have "the source code" from it on my Linux virtual machine, and I was able to cram it into the Bugfix13197 branch on github "Cattlesquat" fork. But that didn't update the PR the way it normally does. I should probably just stop touching anything related to this until you guys figure it out.

@Cattlesquat
Copy link
Collaborator Author

PR #110 has a raw push of what I had on this in the other branch, right as I finally finished and built it the other day.

It's obviously probably got everything "in the wrong directories" and what not. But it's there "just in case".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants