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

Implemented Arrows (#864) #872

Merged
merged 5 commits into from Jun 25, 2016
Merged

Implemented Arrows (#864) #872

merged 5 commits into from Jun 25, 2016

Conversation

RoiEXLab
Copy link
Member

@RoiEXLab RoiEXLab commented Jun 22, 2016

This adresses #864 and fixes some scaling issues (The Route was slightly offset when scaling to extreme values 15% etc.)
Screenshots:
Screenshot 1
Screenshot 2


This change is Reviewable

@RoiEXLab
Copy link
Member Author

Hmm weird... it seems, that we have another flaky test out here:

games.strategy.engine.chat.ChatFloodControlTest > testDeny FAILED

    java.lang.AssertionError

        at org.junit.Assert.fail(Assert.java:86)

        at org.junit.Assert.assertTrue(Assert.java:41)

        at org.junit.Assert.assertFalse(Assert.java:64)

        at org.junit.Assert.assertFalse(Assert.java:74)

        at games.strategy.engine.chat.ChatFloodControlTest.testDeny(ChatFloodControlTest.java:21)

* @param yOffset The vertical pixel-difference between the frame and the Map
* @param scale The scale-factor of the Map
*/
private void drawArrow(Graphics2D graphics, Point2D from, Point2D to, int xOffset, int yOffset, double scale) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you're not hitting any local state, looks like you can mark these private methods as static (note, static on private methods is a good thing since it makes the method easier to understand, provided you do not modify any static state. Static on public methods is bad, creates static coupling that hurts cohesion and makes testing difficult)

@DanVanAtta
Copy link
Member

Some good stuff, but a few things can be improved

@RoiEXLab
Copy link
Member Author

@DanVanAtta done

final Polygon arrowPolygon = new Polygon();
// 1- (-3) = arrowLength
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extract the one to a constant, then you can calculate the -3 value.
Idea is to capture any relationships between lines of code that are known to exist, so everything that needs to change, changes when you change the one thing.

@RoiEXLab
Copy link
Member Author

@DanVanAtta done

@DanVanAtta DanVanAtta merged commit 463b6df into triplea-game:master Jun 25, 2016
@RoiEXLab RoiEXLab deleted the ArrowImplementation branch June 25, 2016 22:14
@RoiEXLab RoiEXLab removed their assignment Dec 10, 2017
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

2 participants