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

Fix CompilerDefines fields #1010

Closed
wants to merge 2 commits into from
Closed

Fix CompilerDefines fields #1010

wants to merge 2 commits into from

Conversation

RblSb
Copy link
Contributor

@RblSb RblSb commented Mar 10, 2019

No description provided.

@RobDangerous
Copy link
Member

RobDangerous commented Mar 10, 2019

That's a very "for my use only" pull request :-P
Let's replace '-' with '_' and prepend '_' to keywords.

@RblSb
Copy link
Contributor Author

RblSb commented Mar 10, 2019

This is more for hxcpp use pr :)
Replacement need field checking for duplicates. Will see how can i check added fields or use StringMap. But you probably don't need if (CompilerDefines._static != null), when you can use #if, same for other bool defines, doesn't? This is why pr is fine.

@RblSb
Copy link
Contributor Author

RblSb commented Aug 28, 2019

HaxeFoundation/haxe#5002 is fixed, so Kha probably broken with current behavior, so i complete this thing.

@RobDangerous
Copy link
Member

I fixed it so much, it will never ever be a problem again (also added an updates.md and referenced this pull request in there).

@RblSb RblSb deleted the patch-1 branch September 15, 2019 12:20
@RblSb
Copy link
Contributor Author

RblSb commented Sep 15, 2019

I mostly interested in this class for window name instead of default:
https://github.com/Kode/Kha/blob/master/Sources/kha/System.hx#L7
So you don't need to set same project name in System.start, only in khafile.js. Can we move Macros.hx to kha.internal and set title = Macros.projectName() as default value?

@RobDangerous
Copy link
Member

I don't think the Window title should be a define, just pass it manually.

@RblSb
Copy link
Contributor Author

RblSb commented Sep 15, 2019

That's sad, then this line should be removed https://github.com/Kode/khamake/blob/master/src/main.ts#L120

@RobDangerous
Copy link
Member

But using that line you can write some code which sets the window title to the project name.

@RblSb
Copy link
Contributor Author

RblSb commented Sep 15, 2019

But you say that this should not be a define :) If you fine with define in khamake then i'm good, but i can also add this define manually in khafile, without khamake dependency.

@RobDangerous
Copy link
Member

It's already there and today I don't feel like removing more things.

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.

2 participants