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

Remove the USING_NS_AX and NS_AX macros. #2103

Merged
merged 1 commit into from
Aug 27, 2024

Conversation

j-jorge
Copy link
Contributor

@j-jorge j-jorge commented Aug 26, 2024

Similar to #2100, this PR removes some unnecessary macros, namely USING_NS_AX and NS_AX.

A comment from @smilediver in the aforementioned PR suggests to deprecate these macros and keep their definition (while still removing their usage). I don't know of any way to mark a macro as deprecated, so I'm afraid that nothing would prevent a programmer to not use them except a line in the release notes. On the other hand I see how such a radical removal could break user code. I have no preference so I would need an authority to tell me if I should restore the definitions :)

@rh101
Copy link
Contributor

rh101 commented Aug 27, 2024

USING_NS_AX is a convenience macro that would currently be used in most projects, so this change is definitely going to break things. The macro itself should not be used within the engine code, but it may be better to keep the define as it is for the sake of developers using it.

NS_AX is a pointless macro, and definitely should be removed.

With NS_AX_BEGIN and NS_AX_END, the primary use was in the engine code, and it would be rare to use these in a game project, so there is no point for them to remain defined at all. If a game project was using them, then the impact would be minor, as I can't imagine why a developer would want to add too many things to the axmol namespace in their project.

@j-jorge
Copy link
Contributor Author

j-jorge commented Aug 27, 2024

Thanks @rh101 :) I have updated the PR following your feedback.

@halx99 halx99 added this to the 2.1.6 milestone Aug 27, 2024
@halx99 halx99 merged commit b436da4 into axmolengine:dev Aug 27, 2024
15 checks passed
@rh101
Copy link
Contributor

rh101 commented Aug 27, 2024

Just FYI, section missed for NS_AX in Macros.h:
image

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