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 support for the creation of layers that are set to invisible in tilemap #2321

Merged
merged 1 commit into from
Jan 10, 2025

Conversation

rh101
Copy link
Contributor

@rh101 rh101 commented Jan 10, 2025

Describe your changes

The current implementation does not allow the creation of layer nodes from layers that are set to invisible in the tilemap file. There is no way to create them, so functionality has been added to allow invisible layers to be created.

The default option is that they are not created, so it does not change existing behavior.

Issue ticket number and link

#2320

Checklist before requesting a review

For each PR

  • Add Copyright if it missed:
    - "Copyright (c) 2019-present Axmol Engine contributors (see AUTHORS.md)."

  • I have performed a self-review of my code.

    Optional:

    • I have checked readme and add important infos to this PR.
    • I have added/adapted some tests too.

For core/new feature PR

  • I have checked readme and add important infos to this PR.
  • I have added thorough tests.

@rudiHammad
Copy link

That works for me . Thanks

@halx99 halx99 merged commit 2212048 into axmolengine:dev Jan 10, 2025
15 checks passed
@rh101 rh101 deleted the tmx-create-invisible-layers branch January 10, 2025 14:59
@rudiHammad
Copy link

rudiHammad commented Jan 10, 2025

Hello again.
I've been testing this a bit more. It does work, but it turns out that if the layer which visibility is off, is used for tile collisions, then this errors. int FastTMXLayer::getTileGIDAt(const Vec2& tileCoordinate, TMXTileFlags* flags /* = nullptr*/).
So don't worry about it, I think everybody else has no issue with this so it's okey.
Sorry about that, I was setting the visibility off in the wrong layers.

@rh101
Copy link
Contributor Author

rh101 commented Jan 10, 2025

Hello again.
I've been testing this a bit more. It does work, but it turns out that if the layer which visibility is off, is used for tile collisions, then this errors. int FastTMXLayer::getTileGIDAt(const Vec2& tileCoordinate, TMXTileFlags* flags /* = nullptr*/).
So don't worry about it, I think everybody else has no issue with this so it's okey.
Sorry about that, I was setting the visibility off in the wrong layers.

You edited your post to add the last sentence, but left everything else in there as it is, so overall it made no sense. If something has changed, then just leave a new comment, but otherwise if you're changing the issue, then use something like the strike-out text option to remove the text that no longer applies, like this, or just some indicator that the rest of the text is no longer valid/no longer applies.

So, from your post above, the only part that is relevant is the "Sorry about that, I was setting the visibility off in the wrong layers.", but what does that mean, is there still an issue or not? If there is a crash, then be more specific about the crash, and include a lot more information, including how to reproduce it.

@rudiHammad
Copy link

rudiHammad commented Jan 10, 2025

In the last sentence I meant to say: "sorry about that, there is still the same issue, but I thought it was solved because I was testing the wrong layers. "
Sorry if I wasn't clear.

I don't want to be a bother with this. So we can close this issue and leave as it was. Meaning as it was by default before the changes.

@rh101
Copy link
Contributor Author

rh101 commented Jan 10, 2025

I don't want to be a bother with this. So we can close this issue and leave as it was. Meaning as it was by default before the changes.

The PR doesn't impact existing behavior, and still gives the developers the option of creating tile layers that are turned off in the input TMX file, so there is no need to roll back the changes.

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.

Saving tilemap (.tmx) having a layer visibility off, makes that layer nullptr in axmol.
3 participants