-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Image HUD element. #5756
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
base: mc-update
Are you sure you want to change the base?
Image HUD element. #5756
Conversation
… an URL/filepath and render it in game.
private static final Identifier LOADING_TEXTURE = Identifier.of(MOD_ID,"textures/icons/gui/loading_image.png"); | ||
private static final Color TRANSPARENT = new Color(255, 255, 255, 255); | ||
private Identifier texture; | ||
private final ExecutorService worker = Executors.newSingleThreadExecutor(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can be Util.getIoWorkerExecutor()
or even virtual thread
private void composeImage(String path) { | ||
// Cancel the future immediately to recreate another texture for the user. | ||
if (currentImageDataFuture != null && !currentImageDataFuture.isDone()) { | ||
currentImageDataFuture.cancel(true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should debounce the modification rather than canceling?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you suggest a custom delay, or for the original image to finish loading?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For example, wait the original value stopped to change for 5 seconds, then we start to load.
|
||
currentImageDataFuture = CompletableFuture.supplyAsync(() -> { | ||
try { | ||
InputStream imageFile = path.startsWith("http") ? new URL(path).openStream() : new FileInputStream(parsed); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might use Http
?
} | ||
} catch (Exception e) { | ||
LOG.debug("Failed to load image", e); | ||
texture = null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here it still accesses field texture
on worker thread. Might move to exceptionallyAsync(..., mc)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or volatile
public int canvasHeight; | ||
public int framesPerColumn; | ||
int totalFrames; | ||
public List<Integer> delays; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can be IntList
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the main difference?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Performance, but it might not be important here.
I updated this PR with the changes you wanted. Lmk if I need to tweak anything else. @MukjepScarlet |
Type of change
New Image element in HUD.
Description
You can select a new element to be rendered in your HUD: Image. It can load images from a filepath / URL and draw them in game. It also has support for GIFs.
Related issues
None
How Has This Been Tested?
Checklist: