This repository has been archived by the owner on Sep 2, 2024. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 28
fix /importfile erroring & update char limit #74
Closed
Closed
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -58,8 +58,8 @@ public void register(Minecraft mc, CommandDispatcher<FabricClientCommandSource> | |
|
||
while (sc.hasNextLine()) { | ||
String line = sc.nextLine(); | ||
if (line.length() > 2000) { | ||
ChatUtil.sendMessage("Line " + (lines.size() + 1) + " is too long! (" + line.length() + " > 2000)", ChatType.FAIL); | ||
if (line.length() > 10_000) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a reason this should be raised to 10,000 specifically? How did you come up with this? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's the new string limit on DiamondFire |
||
ChatUtil.sendMessage("Line " + (lines.size() + 1) + " is too long! (" + line.length() + " > 10,000)", ChatType.FAIL); | ||
continue files; | ||
} | ||
lines.add(line); | ||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
This line used to be in the mod, but we removed it because Minecraft runs in headless mode for a reason. Unless I am mistaken,
/importfile
should use native code, not AWT.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.
Seems like FileDialog is in fact AWT, also from my testing this works perfectly fine, don't see why its an issue
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.
FileDialog is AWT, but we shouldn't be using FIleDialog. It works for you because you are not on a headless machine. For more reference, see how Minecraft implements the "Open Resource Pack Folder" button.
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.
Minecraft uses their own Util class for getting the operating system where they open a folder using a URI command, I don't see how opening a folder is related to picking a file. If it is in fact possible to use a URI command to open a file picker I would love for someone else to work on fixing /importfile since I cannot figure this one out and it's a pretty useful command.
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'm not entirely sure it matters; I'm just going off of what I've looked up and heard in the Fabric discord. I think the idea is that native code can be extended to do more than AWT (since Minecraft used to run on the Raspberry Pi, etc), but for 99.9% of users this isn't an issue. For now I'm just working on other things so I'll leave it to other contributors to debate.