alan412 / LearnJavaForFTC

This is for learning Java for FTC
123 stars 9 forks source link

Rectangles are always all Green #65

Open ftc14169alpha opened 9 months ago

ftc14169alpha commented 9 months ago

Date on version you are commenting on

Jan 2, 2024

Section you are commenting on

16.4

Comments

Line 43 where the code states - Paint nonSelectedPaint = new Paint(selectedPaint);Passing in selectedPaint to the constructor causes it to over ride the red initially set and all rectangles end up Green just doing new Paint() fixes the issue, however, you have to build up the stroke, etc to match the Red box

alan412 commented 9 months ago

I'll update the text to make it more clear. The code to make one of them a different color is in 16.5. In 16.4, it is just drawing the rectangles.

Thanks for your comments to improve the book!

--Alan

On Tue, Jan 2, 2024 at 9:36 PM FTC14169alpha @.***> wrote:

Date on version you are commenting on

Jan 2, 2024 Section you are commenting on

16.4 Comments

Line 43 where the code states - Paint nonSelectedPaint = new Paint(selectedPaint);Passing in selectedPaint to the constructor causes it to over ride the red initially set and all rectangles end up Green just doing new Paint() fixes the issue, however, you have to build up the stroke, etc to match the Red box

— Reply to this email directly, view it on GitHub https://github.com/alan412/LearnJavaForFTC/issues/65, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAF3Y77PFA6OLF2QEYTA4SDYMS73PAVCNFSM6AAAAABBK2DSBKVHI2DSMVQWIX3LMV43ASLTON2WKOZSGA3DGMJWGYZDKNA . You are receiving this because you are subscribed to this thread.Message ID: @.***>

hexaflexa commented 8 months ago

I think when you pass selectedPaint into the Paint() copy constructor for nonSelectedPaint, it ends up modifying selectedPaint to GREEN. This is why the OP sees all three green boxes. I had the same problem.

This problem applies to BOTH Listing 16.5 in Section 16.4 and Listing 16.6 in Section 16.5. (perhaps it would be better to change to 16.4.1 and Listing 16.5.1 to make it less confusing)

As a workaround, this code works:

Paint selectedPaint = new Paint();
selectedPaint.setColor(Color.RED);
selectedPaint.setStyle(Paint.Style.STROKE);
selectedPaint.setStrokeWidth(scaleCanvasDensity * 4);

telemetry.addData("selectedPaint is Red ", selectedPaint.getColor() == Color.RED);

// I this is code is problematic
// Paint nonSelectedPaint = new Paint(selectedPaint);
// nonSelectedPaint.setColor(Color.GREEN);                 // Is this changing selectedPaint to Green as well?!?

// Yes, selectedPaint got changed!
telemetry.addData("selectedPaint is Red ", selectedPaint.getColor() == Color.RED);

// possible workaround
Paint nonSelectedPaint = new Paint();
nonSelectedPaint.setColor(Color.GREEN);
nonSelectedPaint.setStyle(Paint.Style.STROKE);
nonSelectedPaint.setStrokeWidth(scaleCanvasDensity * 4);

I'm not a java expert at all, but the documentation for Paint(Paint paint) implies that the problem we are seeing should NOT occur.

It says "Create a new paint, initialized with the attributes in the specified paint parameter" -- which implies that when you pass a Paint object to a "copy constructor", the new object and the old object should be independent.

However, at least here, it seems like they are not independent, since calling nonSelectedPaint.setColor(Color.GREEN) clobbers the red from the supposedly independent selectedColor. I was very surprised by this, as it doesn't match what I learned a copy constructor should do.

alan412 commented 7 months ago

This is clearly working on my team's robot for this season so I am confused.

https://github.com/ftc16072/CenterStage23-24/blob/master/TeamCode/src/main/java/org/firstinspires/ftc/teamcode/ftc16072/Util/TeamPropDetector.java

Are you running this on a Control Hub? If so, what version?

hexaflexa commented 7 months ago

Hi @alan412, I saw this issue of all three rectangles being green (thus clobbering the original red) when running the code on EOCV-Sim, so maybe the problem stems from there.

I thought I should chime in with more details after seeing the OP @ftc14169alpha report the same problem, but I'm not sure if they were running the code on EOCV-Sim vs an actual Control Hub.

alan412 commented 7 months ago

@hexaflexa - I put this in as a potential issue on EOCV-Sim

serivesmejia commented 3 months ago

Apologies! I became inactive for a while as I was dealing with end-of-highschool stuff. This should be a fairly quick fix, sorry for the overlook.

serivesmejia commented 2 months ago

Fixed in EOCV-Sim v3.4.3

hexaflexa commented 4 weeks ago

The fix in EOCV-Sim 3.4.3 works, so I believe this issue can be closed.