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

Map icons #47

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from
Draft

Map icons #47

wants to merge 6 commits into from

Conversation

jlloh
Copy link
Contributor

@jlloh jlloh commented Oct 24, 2021

Render icons on megamap and minimap.

@@ -284,8 +284,9 @@ func reset_minimap_masks():
compass_corner2 = compass_corner1 + Vector2(compass_width, compass_height)

for minimap_path in $Control/MiniMap.get_children():
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rename. It's not just minimap_path now.

Copy link
Owner

Choose a reason for hiding this comment

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

In the non minimap I have the two types split into different parent nodes.

Burrito/Spatial.gd

Lines 410 to 417 in 55fff1e

for path in paths.get_children():
path.queue_free()
for path2d in minimap.get_children():
path2d.queue_free()
for icon in icons.get_children():
icon.queue_free()

Maybe it makes sense to also do that for the minimap now that it will have icons.

Icon2D.gd Outdated
var texture = Utils.new().generate_texture(texture_path)
self.texture = texture
# TODO: Figure out a better heuristic. Why 0.2?
var scaling_factor = 0.1
Copy link
Owner

Choose a reason for hiding this comment

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

This was the complexity that prevented me from doing this in the previous version. Maybe look at what taco or blish hud does for this? I do not know what the right answer is.

@@ -0,0 +1,16 @@
class_name Utils

func load_image_from_path(path: String):
Copy link
Owner

Choose a reason for hiding this comment

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

Any chance I can convince you to use the same texture loaders for both the icon textures and the path textures? It would be a nice decrease in complexity. Also take a look at #19 to make sure that your solution is compatible with that eventual goal, otherwise the texture loading might need to be rewritten again at a later date.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good suggestion, have attempted to reuse the util function for both paths and icons. Also accept another parameter, RGB vs sRGB and added 2d and 3d textures for paths.

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.

2 participants