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

add files for MakeTextEntry #1039

Merged
merged 5 commits into from
Aug 10, 2023
Merged

Conversation

EntranceJew
Copy link
Contributor

the people, they need strings

@Histalek Histalek added the type/enhancement Enhancement or simple change to existing functionality label Aug 7, 2023
Copy link
Member

@Histalek Histalek left a comment

Choose a reason for hiding this comment

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

I don't really have experience with vgui stuff so i can't really give a proper review on that part. So i'll stick with more general stuff.

Besides addressing my direct comments please also do the following:

  • remove commented out code
  • remove multiple empty lines (there should only be a single empty line at most)
  • double-check the comments, many of them reference a searchbar which doesn't really match the proposed feature of a textarea :D

@EntranceJew
Copy link
Contributor Author

Oops! Overlooked some things, I disabled all the code that shimmed this stuff in from one of my addons, but not the other.
(As an aside, all the stuff defined locally makes it pretty tough to reach and extend the vgui skins.)

The time between reloading the whole gamemode and the F1 menu had me not wanting to touch anything else once it was running since a bad reload on the F1 menu prompted a flashing white panel.

Hopefully this cuts it down to just the bare bones.

Copy link
Member

@Histalek Histalek left a comment

Choose a reason for hiding this comment

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

Codestyle looks good now, thanks!

Don't forget to add an entry to the Changelog :)

@EntranceJew
Copy link
Contributor Author

i set out to change that file and somehow didn't, initially

image
this is what the style looks like now

Copy link
Member

@saibotk saibotk left a comment

Choose a reason for hiding this comment

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

Thank you very much 🙏

Small note: For future PRs please do not resolve conversations on your own, because we have established this as a rule to only let the reviewer close that when they think everything is resolved as they intended. Just because miss understandings happen frequently.

But again Thanks especially for also including a picture 👌

@saibotk saibotk merged commit 4867c8d into TTT-2:master Aug 10, 2023
3 checks passed
@EntranceJew EntranceJew deleted the feature/MakeTextEntry branch August 10, 2023 20:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/enhancement Enhancement or simple change to existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants