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

fix #447 NPE #449

Merged
merged 1 commit into from
Mar 10, 2024
Merged

fix #447 NPE #449

merged 1 commit into from
Mar 10, 2024

Conversation

shitlime
Copy link
Contributor

Fixes #447

Turn on the HandRestock function of Tweakeroo, dig a block in survival mode, such as dirt, and then put it down immediately, it will cause a crash.

@emilyploszaj
Copy link
Owner

I'd prefer to actually resolve the underlying issue here with panels being null if possible, perhaps addWidgets should exit much earlier if EMI is in an error state because of tweakeroo?

@shitlime
Copy link
Contributor Author

I'd prefer to actually resolve the underlying issue here with panels being null if possible, perhaps addWidgets should exit much earlier if EMI is in an error state because of tweakeroo?

You mean like this?

	public static void addWidgets(Screen screen) {
		SidebarPanel panel = panels.get(1);
		if (panel.space == null) {
			EmiLog.warn("panel.space is null, stop addWidgets.");
			return;
		}
		forceRecalculate();
		if (EmiConfig.centerSearchBar) {
			search.x = (screen.width - 160) / 2;
			search.y = screen.height - 21;
			search.setWidth(160);
		} else {
			search.x = panel.space.tx;
			search.y = screen.height - 21;
			search.setWidth(panel.space.tw * ENTRY_SIZE);
		}
		EmiPort.focus(search, false);

		emi.x = 2;
		emi.y = screen.height - 22;
		tree.x = 24;
		tree.y = screen.height - 22;
		updateSidebarButtons();
	}

@shitlime
Copy link
Contributor Author

I'd prefer to actually resolve the underlying issue here with panels being null if possible, perhaps addWidgets should exit much earlier if EMI is in an error state because of tweakeroo?

You mean like this?

	public static void addWidgets(Screen screen) {
		SidebarPanel panel = panels.get(1);
		if (panel.space == null) {
			EmiLog.warn("panel.space is null, stop addWidgets.");
			return;
		}
		forceRecalculate();
		if (EmiConfig.centerSearchBar) {
			search.x = (screen.width - 160) / 2;
			search.y = screen.height - 21;
			search.setWidth(160);
		} else {
			search.x = panel.space.tx;
			search.y = screen.height - 21;
			search.setWidth(panel.space.tw * ENTRY_SIZE);
		}
		EmiPort.focus(search, false);

		emi.x = 2;
		emi.y = screen.height - 22;
		tree.x = 24;
		tree.y = screen.height - 22;
		updateSidebarButtons();
	}

After testing, this way of writing will cause the backpack search bar to be rendered to the upper left corner when it is first opened.

Then, due to my limited level, I did not find the specific reason why panels became null.

@emilyploszaj
Copy link
Owner

forceRecalculate(); should be setting up the panels, and only seems to bail if EmiScreenBase.getCurrent() is null, meaning there's no "real" screen open. This pattern is used in several places in EmiScreenManager and can be put at the start of this method instead. No need to warn, this is an expected case.

EmiScreenBase base = EmiScreenBase.getCurrent();
if (base == null) {
	return;
}

@emilyploszaj
Copy link
Owner

Just want to make sure this actually fixed the problem, and I am going to nitpick, the codestyle of the repo doesn't have single line ifs. If this works and you make the style change I'll happily merge and release for all versions in the next update. Thanks for reporting, debugging, and fixing this issue :)

@shitlime
Copy link
Contributor Author

shitlime commented Mar 2, 2024

Just want to make sure this actually fixed the problem, and I am going to nitpick, the codestyle of the repo doesn't have single line ifs. If this works and you make the style change I'll happily merge and release for all versions in the next update. Thanks for reporting, debugging, and fixing this issue :)

This solves the problem nicely. And thank you very much for reminding.

@emilyploszaj emilyploszaj changed the base branch from 1.20 to 1.20.4 March 2, 2024 17:54
@emilyploszaj
Copy link
Owner

Oh you were targetting the 1.20 branch, but all commits need to be merged into the main 1.20.4 branch and retargetting has caused a really big diff... can you remake the PR based on 1.20.4? I'm sorry for all the work I'm asking of you for such a simple change but I can't merge it without that 😅

@emilyploszaj
Copy link
Owner

I didn't see this was resolved, sorry for the late merge.

@emilyploszaj emilyploszaj merged commit 393d353 into emilyploszaj:1.20.4 Mar 10, 2024
@shitlime shitlime deleted the bugfix/1 branch March 10, 2024 06:46
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.

[BUG] NPE
2 participants