Update draftable edition sections #6631
Conversation
Ayora29
commented
Nov 25, 2024
Ayora29
commented
- FileSection.java : Use a LinkedHashMap to parse edition section, preserving section order and improve performance.
- CardEdition.java : Increment art index when multiple cards with the same name are in the same sheet. Add new section names, remove irrelevant ones.
- Edition txt files of draftable sets : update "sections" and "Booster=" parts to better reflect reallity (cf scryfall and mtg.fandom.com) and reduce amount of unecessary additionnal sections/sheets.
- printsheets.txt : remove unecessary or dead sections
- No card addition, no card remove, no cards in different order compared to before (exept some basic lands)
- I have tested drafts for all editions that I have modified
Add "scheme" section Add fallback sheet in BoosterGenerator Rewrite FileSection and Use LinkedHashMap for sections map
Merge master into edition_sections branch
|
Is this branch now ready? Can someone please test and confirm/approve? Leaving it open for a long time may result in a similar effect as before (the need to merge + some difficult to track issues) |
|
I didn't think those changes to MH3 edition were causing the crash so I've just tested again and it's still crashing for me: main > java.lang.IndexOutOfBoundsException: Index 0 out of bounds for length 0 |
|
Hmm, probably worth checking out in the debugger which file it's unhappy about in particular... I'm only going to be able to do this on the weekend, so hopefully someone is faster, hehe :) |
|
@paulsnoops @Agetian Ok, you should have a blank file that I don't have and it is causing the Index out of bounds Execption. |
| if(!result.containsKey(section)) { | ||
| result.put(section, new ArrayList<>()); | ||
| if(null != lines && !lines.isEmpty()){ | ||
| do{ |
There was a problem hiding this comment.
Why is this a do/while instead of a for loop?
for(int lineNumber = 0; lineNumber < lines.size(); lineNumber++) {}
There was a problem hiding this comment.
it is code optimization. it allows to do the same thing with less line of code and less if statetemens inside the loop.
There was a problem hiding this comment.
I worry that you embed the increment here. lineNumber++ which might confuse people of what is actually going on, and if someone decides to add code that interacts with lineNumber after this increment, it won't be the same value as what gets used on the line itself. Your structure is definitely better than before, but i think getting rid of the forloop actually adds unintended complexity for the next developer.
if(null == lines) {
return result;
}
for(int lineNumber = 0; lineNumber<lines.size(); lineNumber++) {
String line = lines.get(lineNumber);
if (line.startsWith("[") && line.endsWith("]")) {
section = line.substring(1, line.length() - 1);
if(!result.containsKey(section)) {
result.put(section, new ArrayList<>());
}
}
else if(null != section && !line.isEmpty() && !line.startsWith("#")){
result.get(section).add(line);
}
}In fact, we don't even care about the line number, so we could just do...
if (lines == null) {
return result;
}
for(String line : lines) {
if (line.startsWith("#") continue;
if (line.startsWith("[") && line.endsWith("]")) {
section = line.substring(1, line.length() - 1);
if(!result.containsKey(section)) {
result.put(section, new ArrayList<>());
}
} else if(null != section && !line.isEmpty()){
result.get(section).add(line);
}
}There was a problem hiding this comment.
This also seems like we're potentially adding lines to sections that can be just spaces now? since we aren't trimming or string comparison equivilants of that. I didn't even notice what the old function looked like since you had moved the function up the file, and the diff wasn't side by side.
It loads fine now, thanks for supporting custom editions |
