Idiot proofing

Mostly in GhostwriterEditBookScreen, trying to prevent out of bounds crashes.

We're also now enforcing the title length restriction everywhere (found in SharedConstants)

Author: waffle-stomper

src/main/java/wafflestomper/ghostwriter/GhostLayer.java

@@ -44,19 +44,41 @@ public class GhostLayer {
 
 	// These are mostly used for loaded books
 	// Note that EditBookScreen has its own bookTitle field that we need to keep track of
-	public String bookTitle = "";
-	public String bookAuthor = "";
+	private String bookTitle = "";
+	private String bookAuthor = "";
 
 	private boolean buttonsInitialized = false;
 
 
 	public GhostLayer(IGhostBook parent, Screen screenParent, boolean bookIsEditable) {
 		this.parent = parent;
-		this.screen = screenParent;  // TODO: Is there a cleaner way to do this?
+		this.screen = screenParent;
 		this.bookIsEditable = bookIsEditable;
 	}
 
 
+	public String getBookAuthor(){
+		return this.bookAuthor;
+	}
+	
+	
+	public String getBookTitle(){
+		return this.bookTitle;
+	}
+	
+	
+	/**
+	 * Set the internal bookTitle field, enforcing the maximum length from SharedConstants
+	 */
+	public void setBookTitle(String title){
+		if (title.length() < SharedConstants.BOOK_TITLE_MAX_LEN) {
+			this.bookTitle = title;
+		} else {
+			this.bookTitle = title.substring(0, SharedConstants.BOOK_TITLE_MAX_LEN);
+		}
+	}
+	
+	
 	/**
 	 * Called by GhostwriterLecternScreen and GhostwriterReadBookScreen
 	 * Extracts title and author
@@ -65,7 +87,7 @@ public void extractTitleAuthor(ItemStack bookStack) {
 		if (bookStack == null) return;
 		CompoundNBT compoundnbt = bookStack.getTag();
 		if (compoundnbt == null) return;
-		this.bookTitle = compoundnbt.getString("title");
+		this.setBookTitle(compoundnbt.getString("title"));
 		this.bookAuthor = compoundnbt.getString("author");
 	}
 
@@ -192,7 +214,7 @@ private void addSignaturePages() {
 		FileHandler fh = new FileHandler(clip);
 		File sigFile = new File(Ghostwriter.FILE_HANDLER.getSignaturePath(), "default.ghb");
 		if (fh.loadBook(sigFile) && clip.bookInClipboard) {
-			this.parent.insertNewPage(this.parent.getBookPageCount(), "");  // TODO: Is this necessary?
+			this.parent.insertNewPage(this.parent.getBookPageCount(), "");
 			Ghostwriter.GLOBAL_CLIPBOARD.miscPages.addAll(clip.pages);
 			pasteMultiplePages(this.parent.getBookPageCount() - 1);
 			Ghostwriter.PRINTER.gamePrint(Printer.GRAY + "Signature pages added");
@@ -260,7 +282,7 @@ private void collapseTop() {
 	 * Copies a book from the clipboard into the 'real' book
 	 */
 	public void clipboardToBook(Clipboard fromBook) {
-		this.bookTitle = fromBook.title;
+		this.setBookTitle(fromBook.title);
 		this.parent.setBookTitle(this.bookTitle);
 		this.parent.replaceBookPages(fromBook.pages);
 		if (this.parent.getBookPageCount() == 0) this.parent.insertNewPage(0, "");
@@ -364,14 +386,6 @@ public void updateButtons() {
 		}
 
 		this.buttonDisableAutoReload.active = this.autoReloadFile != null;
-		
-		// TODO: Is there a better place to do this?
-		// Trim book title to a max of 32 characters. Anything longer an the book will be marked invalid by
-		// the client when you try to read it
-		// updateButtons() is called when the 'sign' button is clicked, so it's a convenient time to check this
-		if (this.bookTitle.length() > 32) {
-			this.bookTitle = this.bookTitle.substring(0, 32);
-		}
 	}
 
 

src/main/java/wafflestomper/ghostwriter/GhostwriterEditBookScreen.java

@@ -38,7 +38,7 @@ public GhostwriterEditBookScreen(PlayerEntity editingPlayer, ItemStack book, Han
 				() -> this.bookTitle,
 				(p_238772_1_) -> {
 					this.bookTitle = p_238772_1_;
-					this.ghostLayer.bookTitle = this.bookTitle;
+					this.ghostLayer.setBookTitle(this.bookTitle);
 				},
 				this::func_238773_g_,  // getClipboardText
 				this::func_238760_a_,  // setClipboardText
@@ -214,30 +214,50 @@ public boolean isBookBeingSigned() {
 
 	@Override  // From IGhostBook
 	public void insertNewPage(int atPageNum, String pageText) {
-		// TODO: idiot proofing
+		// Idiot proofing
+		if (atPageNum >= SharedConstants.MAX_BOOK_PAGES){
+			Ghostwriter.LOG.error("Cannot insert a page at index " + atPageNum + ". It would make the book too long");
+			return;
+		}
+		
+		// Add blank pages if necessary to prevent IndexOutOfBoundsException
+		while(this.bookPages.size() < atPageNum) this.bookPages.add("");
+		
 		this.bookPages.add(atPageNum, pageText);
 		this.bookChanged(true);
 	}
 
 
 	@Override  // From IGhostBook
 	public void setPageText(int pageNum, String pageText) {
-		// TODO: idiot proofing
+		// Idiot proofing
+		if (pageNum < 0 || pageNum > this.bookPages.size() - 1){
+			Ghostwriter.LOG.error("Couldn't set text on page " + pageNum + " because it doesn't exist");
+			return;
+		}
+		
 		this.bookPages.set(pageNum, pageText);
 		this.bookChanged(true);
 	}
 
 
 	@Override  // From IGhostBook
 	public String getPageText(int pageNum) {
-		// TODO: idiot proofing
+		// Idiot proofing
+		if (pageNum < 0 || pageNum > this.bookPages.size() - 1) return "";
+		
 		return this.bookPages.get(pageNum);
 	}
 
 
 	@Override  // From IGhostBook
 	public void removePage(int pageNum) {
-		// TODO: idiot proofing
+		// Prevent IndexOutOfBoundsException
+		if (pageNum > this.bookPages.size() - 1){
+			Ghostwriter.LOG.error("Can't remove page " + pageNum + ". It doesn't exist");
+			return;
+		}
+		
 		this.bookPages.remove(pageNum);
 		// Add a blank page if the book is empty
 		if (this.bookPages.size() == 0) this.bookPages.add("");
@@ -247,7 +267,6 @@ public void removePage(int pageNum) {
 
 	@Override  // From IGhostBook
 	public void replaceBookPages(List<String> newPages) {
-		// TODO: idiot proofing
 		this.bookPages.clear();
 		this.bookPages.addAll(newPages);
 		if (this.bookPages.isEmpty()) this.bookPages.add("");
@@ -276,7 +295,12 @@ public int getCurrPage() {
 
 	@Override  // From IGhostBook
 	public void setCurrPage(int pageNum) {
-		// TODO: idiot proofing
+		// Idiot proofing
+		if (pageNum < 0 || pageNum > this.bookPages.size() - 1){
+			Ghostwriter.LOG.error("Couldn't move to page " + pageNum + ". It doesn't exist");
+			pageNum = 0;
+		}
+		
 		this.currPage = pageNum;
 		this.bookChanged(false);
 	}

src/main/java/wafflestomper/ghostwriter/GhostwriterFileBrowserScreen.java

@@ -127,8 +127,8 @@ public void init() {
 					new StringTextComponent("filename"));
 			this.filenameField.setMaxStringLength(100);
 			// Add default filename to filenameField
-			String file_title = this.parent.bookTitle;
-			String file_author = this.parent.bookAuthor;
+			String file_title = this.parent.getBookTitle();
+			String file_author = this.parent.getBookAuthor();
 
 			file_title = file_title.trim().replaceAll(" ", ".").replaceAll("[^a-zA-Z0-9.]", "");
 			file_author = file_author.trim().replaceAll(" ", ".").replaceAll("[^a-zA-Z0-9.]", "");
@@ -203,7 +203,6 @@ public void navigateUp() {
 	}
 
 
-	// TODO: This could probably use some more checks
 	private boolean isFilenameValid() {
 		String fn = this.filenameField.getText();
 		return !fn.equals("");