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

Improve the default templates (at least the cpp template) #1494

Merged
merged 16 commits into from
Dec 11, 2023
Merged

Improve the default templates (at least the cpp template) #1494

merged 16 commits into from
Dec 11, 2023

Conversation

aismann
Copy link
Contributor

@aismann aismann commented Dec 10, 2023

Which branch your pull-request should merge into?

  • dev: Current 2.x BugFixs or Features

Describe your changes

see #1493 and diff

Issue ticket number and link

#1493

Checklist before requesting a review

  • I have performed a self-review of my code.
  • If it is a core feature, I have added thorough tests.
  • I have checked readme and add important infos to this PR (if it needed).
  • I have added/adapted some tests too.

@aismann
Copy link
Contributor Author

aismann commented Dec 10, 2023

@halx99 ,
Merging this PR can be the startpoint for the single template with Lua/CPP

@paulocoutinhox
Copy link
Contributor

paulocoutinhox commented Dec 10, 2023

To me this PR make sense, but the name CPPTemplate is horrible to me. You put CPPTemplate everywhere.

Other problem is that changing only this package/bundle dont work, because org.axmol.hellocpp is replaced in other places in axproj-template.json.

Between CPPTemplate and hellocpp i prefer hellocpp.

Other thing is that you need follow the cases where it is lowecase and uppercase.

@aismann
Copy link
Contributor Author

aismann commented Dec 10, 2023

To me this PR make sense, but the name CPPTemplate is horrible to me. You put CPPTemplate everywhere.

Other problem is that changing only this package/bundle dont work, because org.axmol.hellocpp is replaced in other places in axproj-template.json.

Between CPPTemplate and hellocpp i prefer hellocpp.

Other thing is that you need follow the cases where it is lowecase and uppercase.

Why is 'hellocpp' better?

@rh101
Copy link
Contributor

rh101 commented Dec 10, 2023

@aismann As @paulocoutinhox mentioned in his post, the "HelloCpp" and "hellocpp" are placeholders that are replaced when a new Axmol project is created. If you're going to update the name of the project to "CPPTemplate", then you must also change this file to make sure it can look up the correct strings for replacement:

https://github.com/axmolengine/axmol/blob/dev/templates/cpp-template-default/axproj-template.json

@paulocoutinhox
Copy link
Contributor

And if you want to change HelloCpp, I suggest you use a better and more definitive name then, like "AxmolCpp". Something that doesn't look like a hello world or template. I think it's strange to have something written "template" in my project right away.

@rh101
Copy link
Contributor

rh101 commented Dec 10, 2023

And if you want to change HelloCpp, I suggest you use a better and more definitive name then, like "AxmolCpp". Something that doesn't look like a hello world or template. I think it's strange to have something written "template" in my project right away.

Please avoid using the word "Axmol" in the project name, or anything that needs to be replaced, as it makes it more difficult to search for that string, since so much has "Axmol" in it already.

In fact, there is nothing wrong with HelloCpp at all. It's a placeholder, and as such, it is always going to be replaced by the developer, so all it needs to be is unique enough to notice, and easy enough to search for.

Also, has anyone ever actually logged an issue related to "HelloCpp", and I don't mean an issue related to personal preference, but a specific concern or technical issue about its usage? If not, then perhaps putting much focus on changing it is of no real benefit to developers using Axmol.

@aismann
Copy link
Contributor Author

aismann commented Dec 10, 2023

And if you want to change HelloCpp, I suggest you use a better and more definitive name then, like "AxmolCpp". Something that doesn't look like a hello world or template. I think it's strange to have something written "template" in my project right away.

  1. 'CPPTemplate/HelloCpp' will be replaced on axmol new...
  2. 'CPPTemplate/HelloCpp' are only placeholders and will add cpp-template-default into project(axmol) for tmp test on e.g.: VSStudio.
    image

Also, has anyone ever actually logged an issue related to "HelloCpp", and I don't mean an issue related to personal preference, but a specific concern or technical issue about its usage? If not, then perhaps putting much focus on changing it is of no real benefit to developers using Axmol.

  1. My reason to rename 'HelloCpp' is:
    image

@paulocoutinhox
Copy link
Contributor

Cannot be "CppProject", "MyProject", "MyApp", "CppApp", something more beautiful?

@aismann
Copy link
Contributor Author

aismann commented Dec 10, 2023

Cannot be "CppProject", "MyProject", "MyApp", "CppApp", something more beautiful?

No. Its only for:
add cpp-template-default into project(axmol) for tmp test
=> NOT for your project (which is mostly wrong used of course)
Hint: Changing anything on 'HellpCpp/HelloLua' had changed also the template itself on your axmol folder

@paulocoutinhox
Copy link
Contributor

The problem is that you are changing everything for CPP, you need do like me on latests PR, that is change for BOTH templates.

You changing all for this, but lua template follow the same pattern, ex: HelloLua, org.axmol.hellolua.

The more you keep messing with it, the harder it is to maintain two templates. I've already spent a lot of time adjusting both projects and now you're going to make them different again.

@aismann
Copy link
Contributor Author

aismann commented Dec 10, 2023

The more you keep messing with it, the harder it is to maintain two templates. I've already spent a lot of time adjusting both projects and now you're going to make them different again.

Sorry. you should not using axmol which is continues improving ;)
@paulocoutinhox if you familiar with Lua maybe you can write a PR which makes one template for cpp/lua (using this PR as start point)?

@rh101
Copy link
Contributor

rh101 commented Dec 10, 2023

Sorry. you should not using axmol which is contininus improving ;)

Improvements to Axmol are great, but, in order for developers to use Axmol, it needs to be relatively stable, in terms of frequency of changes, and the impact that the changes have. Stating that a developer should not be using Axmol because it's continuously improving is actually a negative, not a positive, and perhaps a little inappropriate too, don't you think?

@aismann
Copy link
Contributor Author

aismann commented Dec 10, 2023

Sorry. you should not using axmol which is contininus improving ;)

Improvements to Axmol are great, but, in order for developers to use Axmol, it needs to be relatively stable, in terms of frequency of changes, and the impact that the changes have. Stating that a developer should not be using Axmol, because it's continuously improving is actually a negative, not a positive, and perhaps a little inappropriate too, don't you think?

No I think not. Everybody can use own projects which was created before whithout an effect there.
A new template is part of the improvement of axmol. A sentence like I've already spent a lot of time adjusting both projects and now you're going to make them different again is funny.

Sorry. you should not using axmol which is contininus improving ;)

Was also mean as a funny sentence of course (look on the smiley)

@aismann
Copy link
Contributor Author

aismann commented Dec 10, 2023

@halx99
Im finished, Can be merged.

@aismann
Copy link
Contributor Author

aismann commented Dec 10, 2023

Ok. I see neverone understand what 'HelloCpp' realy is:
Its not an one project!
Its always using the sources of cpp-default-template.
So CPPTemplate is more correct.
Btw changing anything on 'HelloCpp' was always changing the cpp-default-template content not a 'HelloCpp' stuff.
An old Cocos2d-x mistake which will be correct now.
Thats my point of view

@halx99
Copy link
Collaborator

halx99 commented Dec 10, 2023 via email

@aismann
Copy link
Contributor Author

aismann commented Dec 10, 2023

HelloCpp lets developer meaning thats an own project and not the template itself.

HelloCpp should never used as a full project.
See the comment on CMakeList too:
add cpp-template-default into project(axmol) for tmp test

axmol new... wil be affected

@rh101
Copy link
Contributor

rh101 commented Dec 10, 2023

@aismann I'm sure everyone understands that it is just a temporary project name for the template and nothing more. There is no need to force the issue.

There are a number of items in this PR that are actual good to change, such as the HelloWorldScene change to MainScene etc., so best to enable them to be merged in sooner rather than later by limiting this PR to only those useful changes for now.

The project name change can be given more consideration, and can always be changed in a future PR if the need arises.

@aismann
Copy link
Contributor Author

aismann commented Dec 11, 2023

Should be ready for merge now

@halx99 halx99 linked an issue Dec 11, 2023 that may be closed by this pull request
@halx99 halx99 added this to the 2.1.0-LTS milestone Dec 11, 2023
@halx99 halx99 added the enhancement New feature or request label Dec 11, 2023
@halx99 halx99 merged commit 62e6542 into axmolengine:dev Dec 11, 2023
9 checks passed
@aismann aismann deleted the improve-CPPTemplate branch December 11, 2023 06:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve the default templates (at least the cpp template)
4 participants