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

versioned atlas #1281

Merged
merged 4 commits into from
Jan 8, 2022
Merged

versioned atlas #1281

merged 4 commits into from
Jan 8, 2022

Conversation

kianzarrin
Copy link
Collaborator

@kianzarrin kianzarrin commented Jan 8, 2022

old code was trying to destroy old atlases to enable hot-reload. but it had 2 problems:

  • the logic of the code was such that if two buttons need the same atlas, the new button destroyed the atlas of the prev button (in debug builds only). as a result the remove vehicle button was not rendered.
  • Also its not clear to me if destroying the atlas was enough because the textures are still in memory ... right? so there should still be memory leak.

changes:

  • The new code adds version to atlas name and deals with the hot-reload issue that way.
  • I also introduced Ingame property in TextureUtil to avoid type mistake when getting Ingame atlas.
  • I didn't try to solve the memory leak issue because it could get complicated and should be topic of another PR.

TMPE.zip

@kianzarrin kianzarrin added UI User interface updates lifecycle related to the loading process like enabling/disabling/loading/reloading/load order/hot reload labels Jan 8, 2022
@kianzarrin kianzarrin self-assigned this Jan 8, 2022
@kianzarrin
Copy link
Collaborator Author

now its working :)
image

Copy link
Collaborator

@kvakvs kvakvs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for investigating and solving this

TLM/TLM/UI/RemoveVehicleButtonExtender.cs Outdated Show resolved Hide resolved
TLM/TLM/U/TextureUtil.cs Show resolved Hide resolved
Copy link
Member

@krzychu124 krzychu124 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@kianzarrin kianzarrin merged commit 1ace058 into master Jan 8, 2022
@kianzarrin kianzarrin deleted the remove-car-button branch January 8, 2022 22:37
@originalfoo originalfoo added this to the 11.6.0 milestone Jan 14, 2022
@originalfoo originalfoo modified the milestones: 11.6.0, 11.6.3 Jan 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lifecycle related to the loading process like enabling/disabling/loading/reloading/load order/hot reload UI User interface updates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants