Skip to content

Conversation

@ssarmis
Copy link
Contributor

@ssarmis ssarmis commented Feb 11, 2018

Changed the name color due to low visibility on sand;
Fixed the position for the username text and cached the divisions.

Fixed the position for the username text and cached the divisions.
this.colour = Colours.get(-1, 111, shirtCol, faceCol);
fireRate = Small.FIRE_RATE;
// Perfectly matches the text over the head
nameOffset = (userName.length() / 2) * fontCharSize - ((userName.length() & 1) == 0 ? fontCharSize / 2 : 0);
Copy link
Contributor

Choose a reason for hiding this comment

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

Might consider splitting the equation from the ternary for readability.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right about that, didn't observe its over 80 characters long.

private int fireRate = 0;

// Need to add a class for constants
private int fontCharSize = 8;
Copy link
Contributor

Choose a reason for hiding this comment

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

Could probably be "final" :-)

@DylanMeeus
Copy link
Contributor

Overall good commit (code wise), didn't yet check it in the game itself though!
👍

@redomar
Copy link
Owner

redomar commented Feb 11, 2018 via email

@redomar
Copy link
Owner

redomar commented Feb 12, 2018

Thoughts

I agree it is difficult to see the name tag over the sandy areas, An excellent idea.

Issue I found

The code works only if you input a desired username, however if you blast through without typing a username it isnt aligned at all. Guest names are generated only when the username box is empty.
screen shot 2018-02-12 at 12 13 09
screen shot 2018-02-12 at 12 16 50

My solution

However, I have a solution moving This line into this part. I'm guessing it is because it is after the 'temporary/guest' name has been made.

What do you think

Do you know why this is happening? Do you have another way to deal with this or are you going to try my solution, I'm open to opinions.

@ssarmis
Copy link
Contributor Author

ssarmis commented Feb 12, 2018

Yes, this is because the username initially is blank and gets modified on the go to be replaced with the "guest" one.
Its an easy fix, just need to change the offset when the username gets generated after it was blank, I will upload it in a sec.

@ssarmis
Copy link
Contributor Author

ssarmis commented Feb 12, 2018

It looks a bit ugly (2 lines of code are repeating, could just use a function) + if the name is only 1 character long, its going to recalculate the offset again, but its perfectly aligned over the head.

@redomar
Copy link
Owner

redomar commented Feb 12, 2018

Thanks for your time and effort

@redomar redomar merged commit 7d5f7b4 into redomar:master Feb 12, 2018
@ssarmis ssarmis deleted the usernamePositionFix-changedNameColor branch February 12, 2018 17:41
@ssarmis
Copy link
Contributor Author

ssarmis commented Feb 12, 2018

No problem, the pleasure is mine.

@redomar
Copy link
Owner

redomar commented Feb 12, 2018

I messed up sorry, would you like to push the changes again, I overwritten the master directory when I was merging from previous work

@redomar
Copy link
Owner

redomar commented Feb 12, 2018

I've not used GitHub/Git in ages and I foolishly pushed a bad merge that was old, And 'reverting merge' opens a new pull request which I didn't know

@ssarmis ssarmis restored the usernamePositionFix-changedNameColor branch February 13, 2018 01:25
@ssarmis
Copy link
Contributor Author

ssarmis commented Feb 13, 2018

I made another pr to be easier.

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.

3 participants