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

Update TileMap to use Vector2i #39976

Merged
merged 1 commit into from
May 13, 2021
Merged

Conversation

aaronfranke
Copy link
Member

@aaronfranke aaronfranke commented Jun 30, 2020

Seems to work well.

scene/2d/tile_map.cpp Outdated Show resolved Hide resolved
@groud
Copy link
Member

groud commented Dec 8, 2020

I am not sure what to do with this PR. I am rewriting the whole TileMap node, which now already uses Vector2i on my side.
Unless we want this merged into 3.2, I don't think it makes a lot of sense to merge this now in master.

@aaronfranke
Copy link
Member Author

@groud There is no Vector2i in 3.x, so it's not possible to merge this in 3.2 :)

The ideal thing would have been to merge this PR before you did the TileMap rewrite, this is the unfortunate reality of what happens when PRs go sitting for months and don't get reviewed.

Go ahead and make whatever changes you need, once those are merged I can check if there's anything missing and if so I can rebase this PR, otherwise I'll close this PR later.

@groud
Copy link
Member

groud commented Dec 8, 2020

The ideal thing would have been to merge this PR before you did the TileMap rewrite, this is the unfortunate reality of what happens when PRs go sitting for months and don't get reviewed.

The work done here would have been wiped out anyway, whether it would have been merged earlier or not would not have changed this.

Go ahead and make whatever changes you need, once those are merged I can check if there's anything missing and if so I can rebase this PR, otherwise I'll close this PR later.

Ok, sounds good to me.

@aaronfranke aaronfranke force-pushed the tilemap-vec2i branch 2 times, most recently from 24f8e99 to 1549d8d Compare May 4, 2021 07:01
@aaronfranke aaronfranke requested a review from a team as a code owner May 4, 2021 07:01
@aaronfranke aaronfranke changed the title Update TileMap to use Vector2i instead of two ints Update TileMap to use Vector2i May 8, 2021
@aaronfranke aaronfranke removed the request for review from reduz May 9, 2021 01:08
@akien-mga akien-mga merged commit a3dd18b into godotengine:master May 13, 2021
@akien-mga
Copy link
Member

Thanks!

@aaronfranke aaronfranke deleted the tilemap-vec2i branch May 13, 2021 13:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants