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

String table empty string #1923

Merged
merged 3 commits into from Jan 26, 2017

Conversation

Projects
None yet
3 participants
@elfprince13
Contributor

elfprince13 commented Jan 12, 2017

I was going to merge some old changes of mine from the 1.0.1 days for having a global "empty stringtable entry", and realized the engine had since added the functionality, but didn't appear to actually be using it anywhere. This PR is a global find and replace with StringTable->insert("") -> StringTable->EmptyString().

@crabmusket

This comment has been minimized.

Show comment
Hide comment
@crabmusket

crabmusket Jan 12, 2017

Contributor

Whitespace 😢

Contributor

crabmusket commented Jan 12, 2017

Whitespace 😢

@elfprince13

This comment has been minimized.

Show comment
Hide comment
@elfprince13

elfprince13 Jan 12, 2017

Contributor
[filter "tabspace"]
	clean = expand -t 3
	smudge = unexpand -t 3

Is a useful thing to prevent my editor from putting in tabs where I don't want them. The downside is...it's a little aggressive about making sure every file complies, and apparently the codebase isn't super consistent.

Contributor

elfprince13 commented Jan 12, 2017

[filter "tabspace"]
	clean = expand -t 3
	smudge = unexpand -t 3

Is a useful thing to prevent my editor from putting in tabs where I don't want them. The downside is...it's a little aggressive about making sure every file complies, and apparently the codebase isn't super consistent.

@Areloch

This comment has been minimized.

Show comment
Hide comment
@Areloch

Areloch Jan 12, 2017

Contributor

First) nice PR
Second) I wouldn't fret super hard on the whitespace cleanup part of it. It looks like it conforms stuff to the 3 space, even if parts aren't especially relevent to this PR in particular. We're planning on doing a whitespace cleanup pass for one of the first parts of the push towards 4.0 when 3.10 goes out, along with guideline compliance and the like.

So yeah, wouldn't fret much on it if you inadvertently catch some whitespace cleanup in the course of things. We'll get it all cleaned and consistent one way or another pushing on towards 4.0 :)

Contributor

Areloch commented Jan 12, 2017

First) nice PR
Second) I wouldn't fret super hard on the whitespace cleanup part of it. It looks like it conforms stuff to the 3 space, even if parts aren't especially relevent to this PR in particular. We're planning on doing a whitespace cleanup pass for one of the first parts of the push towards 4.0 when 3.10 goes out, along with guideline compliance and the like.

So yeah, wouldn't fret much on it if you inadvertently catch some whitespace cleanup in the course of things. We'll get it all cleaned and consistent one way or another pushing on towards 4.0 :)

@elfprince13

This comment has been minimized.

Show comment
Hide comment
@elfprince13

elfprince13 Jan 12, 2017

Contributor

Awesome :)

Contributor

elfprince13 commented Jan 12, 2017

Awesome :)

@Areloch

This comment has been minimized.

Show comment
Hide comment
@Areloch

Areloch Jan 24, 2017

Contributor

Looks like a couple conflicts snuck their way in. Can you double-check those so I can get this merged in? Thanks :)

Contributor

Areloch commented Jan 24, 2017

Looks like a couple conflicts snuck their way in. Can you double-check those so I can get this merged in? Thanks :)

@elfprince13

This comment has been minimized.

Show comment
Hide comment
@elfprince13

elfprince13 Jan 24, 2017

Contributor

Should be all good. It was all NULL -> nullAsType<>() conversions from #1920

Contributor

elfprince13 commented Jan 24, 2017

Should be all good. It was all NULL -> nullAsType<>() conversions from #1920

@Areloch Areloch merged commit 7185d96 into GarageGames:development Jan 26, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment