Added farms (partial fix of the issue #86) #140
Conversation
…mports, added horizontal rule function)
Merge upstream commits
| Ui.print("(" + ((seed == null) ? "No seeds planted)" : seed.getName() + ") - ")); | ||
| Ui.println((seed == null) ? "" : ((this.getRemainingTime() > 0) ? this.getRemainingTime() + " fights remaining until the harvest." : "Ready for harvest")); | ||
| Ui.println(); | ||
| // Can you think of any way I could possibly make this work? I wanted to make the menu dynamic - display the harvest option only if the crops are ready for harvest - but I just can't come up with a universal and scalable way to solve this other than bunch of if-else statements, which is rather bulky and doesn't meet either specified requirement. |
There was a problem hiding this comment.
Here is the first comment.
There was a problem hiding this comment.
Unfortunately I couldn't think of a clean way of doing this without getting into some much more advanced stuff (a seperate helper class that can take an array of functions/items, and dynamically display them in order without any hard-coded number). I think a few if statements is good enough in this case. This is what I got (similar to yours): ``` boolean readyToHarvest = false;
print("1) Plant Seeds");
if (readyToHarvest) print("2) Harvest");
print(readyToHarvest ? "3" : "2" + ") Back");
switch (getValidInt()) {
case 1:
plant();
return;
case 2:
if (readyToHarvest) {
harvest();
return;
} else {
print("quit!");
return;
}
case 3:
if (readyToHarvest) {
print("quit!");
return;
}
default:
print("fallback");
}```
(Done in a sandbox test environment, would need tweaking to work in this codebase)
For the situation where there are an unknown amount of extra options, theres an example of how this could be done here:
although its not really a clean solution either.
If there was to be some nice automated menu-builder classs, that should be done on a seperate PR/issue since it would be a pretty big project & would include converting all the old menus to use the new helper class
| println("-".repeat(hrLength)); | ||
| } | ||
|
|
||
| // This function will reliably centre solely strings of an even length. I don't know of a way to fix this without varying hrLength. Do you? Couldn't varying hrLength solve the issue reliably as well? |
There was a problem hiding this comment.
Here is the second one.
There was a problem hiding this comment.
I don't think there is a way to solve this. A variable hr length could work in theory, but would probably be more of a hassle than its worth, as the printhr function would need to know the string length as well, which would be weird. it isn't really noticeable anyways so I wouldn't worry about it
|
I'll take a look at this over the weekend! been busy over the holidays :) Both are interesting problems, but I think I can come up with solutions |
|
Alright. Thanks. :) |
| import java.util.ArrayList; | ||
|
|
||
| public class Crop { | ||
| public static final ArrayList<Crop> cropArrayList = new ArrayList<Crop>(); |
There was a problem hiding this comment.
this is implicit and not actually needed
| public static final ArrayList<Crop> cropArrayList = new ArrayList<Crop>(); | |
| public static final ArrayList<Crop> cropArrayList = new ArrayList<>(); |
| public class Crop { | ||
| public static final ArrayList<Crop> cropArrayList = new ArrayList<Crop>(); | ||
|
|
||
| private String name; |
There was a problem hiding this comment.
these three variables can be final
| } | ||
|
|
||
| if (firstInit) { | ||
| this.owned = 0; |
There was a problem hiding this comment.
this is already set as 0 in the declaration on line 18
| } | ||
| } | ||
|
|
||
| public void sell(int piecesSold){ |
There was a problem hiding this comment.
| public void sell(int piecesSold){ | |
| public void sell(int piecesSold) { |
| Coins.set(totalPrice, true); | ||
| Stats.coinsGainedOnCrops += totalPrice; | ||
| this.owned -= piecesSold; | ||
| Ui.println("You have sold " + this.getName() + " (" + piecesSold + " bundles)" + " for " + totalPrice + " coins."); |
There was a problem hiding this comment.
You don't have to do it this way, but an alternative could be
Ui.println(String.format("You have sold %s (%s bundles) for %s coins.", this.getName(), piecesSold, totalPrice));
| } | ||
|
|
||
| if (firstInit) { | ||
| this.owned = 0; |
There was a problem hiding this comment.
already set to 0 on declaration
| @@ -1,11 +1,15 @@ | |||
| package com.hotmail.kalebmarc.textfighter.main; | |||
|
|
|||
| import com.hotmail.kalebmarc.textfighter.farm.Farm; | |||
| import com.hotmail.kalebmarc.textfighter.farm.Field; | |||
| import com.hotmail.kalebmarc.textfighter.farm.Farm; | ||
| import com.hotmail.kalebmarc.textfighter.farm.Field; | ||
| import com.hotmail.kalebmarc.textfighter.farm.Seed; | ||
| import com.hotmail.kalebmarc.textfighter.farm.Crop; |
| if (menuItem <= suitableFarms.size()) { | ||
| suitableFarms.get(menuItem - 1).menu(); | ||
| } | ||
| return; |
There was a problem hiding this comment.
this return doesn't do anything, can be removed
| @@ -112,6 +112,16 @@ public static void println() { | |||
| print("\n"); | |||
| } | |||
|
|
|||
| private static int hrLength = 66; | |||
| public static void printhr() { | |||
| println("-".repeat(hrLength)); | |||
There was a problem hiding this comment.
.repeat requires java 11+, which is fine, but could you add a note in the README stating java 11+ is required? :) It's common for java developers to use much older versions like java 8, 9, etc
|
i accidently merged then unmerged this branch lol. If youre still interesting in having it merged, please fix the above comments and reopen this PR :) sorry about that |
|
Hey, sorry. I've been quite busy with school stuff since the New Year. I'll take a look at it as soon as I have a bit of time. |
|
yep, no worries. No rush! |
Hello,
I am adding farms with this pull request. I've got other types of properties - passive income properties, to be exact - in mind but I'll leave those for later. Please focus on the two comments in the code which I added. I'll try to highlight them using code comments.
Thanks in advance,
RindT
(#86)