axmolengine / axmol

Axmol Engine – A Multi-platform Engine for Desktop, XBOX (UWP) and Mobile games. (A fork of Cocos2d-x-4.0)
https://axmol.dev
MIT License
868 stars 195 forks source link

Initial settings different in FastTMXLayer::setupTileSprite() #1283

Closed aismann closed 11 months ago

aismann commented 1 year ago

Fixed FastTMXLayer::setupTileSprite() (changed sprite->setAnchorPoint(Vec2::ZERO); to sprite->setAnchorPoint(Vec2(0.5f, 0.5f)); image @halx99 Can I add this small PR? The problem is: You can see on the screenshot the position is now different. So its also a API change.

Here is a code snippet which work WHITHOUT changing the API: ` void FastTMXLayer::setupTileSprite(Sprite* sprite, const Vec2& pos, uint32_t gid) { // sprite->setPosition(getPositionAt(pos)); sprite->setPositionZ((float)getVertexZForPos(pos)); // sprite->setAnchorPoint(Vec2::ZERO); sprite->setOpacity(this->getOpacity());

// put the anchor in the middle for ease of rotation.
sprite->setAnchorPoint(Vec2(0.5f, 0.5f));
sprite->setPosition(getPositionAt(pos).x + std::roundf(sprite->getContentSize().height / 2),
    getPositionAt(pos).y + std::roundf(sprite->getContentSize().width / 2));

` image

halx99 commented 1 year ago

Yes, you can

rh101 commented 1 year ago

@aismann Is this actually a bug in the current implementation? Or is it just a preferred initial setting for the anchor point?

By "bug" I mean something that is not working as it should. From what I see, the current implementation is working as it should, but you're requesting to change that functionality. Perhaps labeling this as a "bug" is a little misleading, as it's more a change to initial settings of the tile layer.

So, if this is not in fact a bug, but a change to existing behaviour based on preference, then perhaps getting input from existing users prior to the change may be a good idea, because again, this will affect existing games using the tile functionality in Axmol.

aismann commented 1 year ago

@aismann Is this actually a bug in the current implementation? Or is it just a preferred initial setting for the anchor point?

Its look as a bug for me. Compare the screenshots. Andf if you look into FastTMXLayer::setupTileSprite itself you can see with rotation the anchor point is changing too. So this fix set the anchor point for all sprites to the same value.

rh101 commented 1 year ago

Its look as a bug for me. Compare the screenshots.

I don't use tiled functionality, so I don't have an opinion on whether this is classed as a bug or not. My only concern is that a change like this impacts any existing projects using the tiled functionality, and yet no-one has ever logged this issue as a bug, whether for Cocos2d-x or Axmol. That is why I'm questioning the reason for the change.

Any developers who actually use the tiled functionality should consider giving their opinion on the change before it is merged in.

aismann commented 1 year ago

Its look as a bug for me. Compare the screenshots.

I don't use tiled functionality, so I don't have an opinion on whether this is classed as a bug or not. My only concern is that a change like this impacts any existing projects using the tiled functionality, and yet no-one has ever logged this issue as a bug, whether for Cocos2d-x or Axmol. That is why I'm questioning the reason for the change.

Any developers who actually use the tiled functionality should consider giving their opinion on the change before it is merged in.

It woks as before: on the second screenshot I have forgott to adapt the sprite position. See the last screenshot there has the sprites the same position as on the first screenshot

stale[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.