Button error in gradle build

Pesegato · September 14, 2016, 12:21pm

Got the same issue, fixed right away

asser_fahrenholz · September 14, 2016, 12:44pm

Same fix I did locally. I haven’t gotten into the whole fork/fix/pull mechanics yet - but will try it in the future when I have a proposed fix for something

pspeed · September 14, 2016, 2:19pm

Your fix is calling the wrong method. The whole point is to call the method in the outer class as the one in the inner class doesn’t do the right thing anymore.

…with your change, the click events won’t be fired.

The real question is why isn’t Java 8 finding the click() method that is perfectly in scope for the inner class:

github.com

Pesegato/Lemur/blob/36893200cd78bf5c29be18e811d0d4221ca3e72f/src/main/java/com/simsilica/lemur/Button.java#L330


    this.pressed = f;
    if( pressed ) {
        commandMap.runCommands(ButtonAction.Down);
        runEffect(EFFECT_PRESS);
    } else {
        commandMap.runCommands(ButtonAction.Up);
        runEffect(EFFECT_RELEASE);
    }
}
protected void click() {
    if( !isEnabled() )
        return;
    commandMap.runCommands(ButtonAction.Click);
    runEffect(EFFECT_CLICK);
}
@Override
public String toString() {
    return getClass().getName() + "[text=" + getText() + ", color=" + getColor() + ", elementId=" + getElementId() + "]";
}

If Java 8 changed to the scoping rules then it’s really stupid… because protected should be available to both inner classes as well as friendly access to everything else in the package.

But the call you’ve changed it do does nothing:

github.com

Pesegato/Lemur/blob/36893200cd78bf5c29be18e811d0d4221ca3e72f/src/main/java/com/simsilica/lemur/Button.java#L386


            click();
        }
        // Only mapped to one function so no need to distinguish
        setPressed(isEnabled() && value == InputState.Positive);
    }
}
protected class ButtonMouseHandler extends DefaultMouseListener {
    @Override
    protected void click( MouseButtonEvent event, Spatial target, Spatial capture ) {
    }
    @Override
    public void mouseButtonEvent( MouseButtonEvent event, Spatial target, Spatial capture ) {
    
        // Buttons always consume their click events
        event.setConsumed();
    
        // Do our own better handling of 'click' now
        //super.mouseButtonEvent(event, target, capture);

…and so breaks a bunch of stuff. It’s only there to override the superclass’s method.

pspeed · September 14, 2016, 2:57pm

I’ve just checked Button.java with a fix that hopefully works around this bug in the Java 8 compiler. (I can’t see how it’s not a javac bug, anyway.)

Pesegato · September 14, 2016, 3:06pm

I hope this is the proper fix

pspeed · September 14, 2016, 3:11pm

Or try my fix… because it seems to work fine for the other inner class that doesn’t have its own click() method. So I renamed the outer class click() method to runClick() to work around the bug in the Java 8 compiler.

pspeed · September 14, 2016, 3:12pm

My commit:

asser_fahrenholz · September 14, 2016, 8:20pm

Thanks.

MegaWolf · September 14, 2016, 10:14pm

Well I can’t fid any hint whether this is desired or not, but the easier solution woud have been:
Button.this.click();

pspeed · September 14, 2016, 10:27pm

Except it’s ugly and if you’ve ever looked at the byte code, it’s often more instructions.

Easier to work around a compiler bug with a method name change.

MegaWolf · September 15, 2016, 5:40am

More bytecode does not necessarily means that it takes longer.

BTW

for (int i = 0; i < 10; i++) {}
needs 10 bytecodes per iteration whereas

for (int i = 10; --i >= 0; ) {}
needs only 7

Or do you care about the huge more disk place

Edit: Are you even sure about that; I think you have to push this on the stack for every (non-static) method you are using; therefore you have to push the Outerclass’ this on the stack anyway, which is what a button.this does.

pspeed · September 15, 2016, 6:04am

Anyway, it’s cleaner and avoids future issues to just rename the method. Since the Java 7 compiler that I use doesn’t have the bug but the Java 8 compiler does have the bug… we’d hit this again the next time I refactored and forgot to put the extra garbage at the beginning.

I’ll just make a mental note that Java 8’s method resolution code is broken and try to avoid using the same names for inner/outer class methods in the future.

Pesegato · September 15, 2016, 6:27am

I might be wrong, but I think that Outerclass.this is the intended way of referencing methods from the outer class (and by intended I mean by James Gosling). My guess is that the ‘bug’ is on Java7

MegaWolf · September 15, 2016, 6:41am

It’s the intended way yes, but only for methods with the same signature; so same name and same parameters, so normally click() should work (Don’t know what they changed, that it once worked and not now)

pspeed · September 15, 2016, 6:45am

Me either. That has worked since Java 1.2 at least (from personal experience) and Java 8 is the first time I’ve seen it not work.

I’d claim it worked in Java 1.1 but my experience with 1.1 is not extensive enough to be sure that I did that with an inner class. In 1.2 with Swing, I 100% did. So common.

pspeed · September 15, 2016, 6:54am

I tried to look up and see if there is an open bug for this already but there is a lot of “method not found” related churn in Java 8 for different versions. (And u20 seems to have shipped with a few missing bug fixes)

So, what version of JDK 8 are you using that shows this issue?

Pesegato · September 15, 2016, 7:17am

1.8.0_77 (Oracle Corporation 25.77-b03)

Empire_Phoenix · September 15, 2016, 1:36pm

… wait what?
Version 8 Update 101 is current, are you a ubuntu user by chance?

Pesegato · September 15, 2016, 1:56pm

You can see my spec on last screen here Mermaid: Lemur-compatible 3d widgets

I don’t get the craze at getting the latest sdk bugfix update… just wasted time at downloading+installing to me.

sailsman63 · September 15, 2016, 5:17pm

I’ve been able to duplicate this on OpenJDK 1.8.0_102-b14. (What Arch has installed by default if you run system updates regularly)

I’ve been playing with a minimal example, just to see if anything else pops up.

Access level for the method in the outer class is irrelevant. Even public methods will have this compilation error.
Access of method in the Superclass of the inner class will only cause this issue if the method is visible to the subclass, which is about what I would expect.

Sorry if this is beginner stuff, hope it’s at least a little helpful. I have also not been able to find any references to this issue in the openJDK bug tracker.

EDIT: and, you may be already aware, the whole “superclass of inner class” thing is irrelevant. You can have your overload defined In the inner class, and will still have the error when attempting to access the outer class method.