Enhance CLI output formatting and logging#27
Conversation
|
No official review, thus only gonna post a comment here: Im not sure if adding chunkbase/seedmap links to the output will benefit anyone, i think most people never observe the line being printed after CPU eval passes, and just resort checking the output file for found seeds. You seem to create new structs, when you could reuse the existing ones, from Lines 32 to 43 in 3408b3d If your struct is needed i believe it should be placed in the respective header file. Same goes for your added functions. They should be specified in the respective header. |
BoySanic
left a comment
BoySanic
left a comment
There was a problem hiding this comment.
If you resolve these items I think we can merge.
| #include <algorithm> | ||
| #include <random> | ||
| #include <atomic> | ||
|
|
There was a problem hiding this comment.
Don't see why we need the extra newline
| std::optional<HostService> server; | ||
| std::optional<std::string> output_file; | ||
| std::optional<int64_t> start_seed; | ||
|
|
| output_file = argv[i++]; | ||
| } else if (std::strcmp("--start", arg) == 0) { | ||
| if (!parse_argument_int(argc, argv, i, start_seed, [](int64_t start_seed){ return true; }, arg)) return false; | ||
|
|
|
|
||
| void print_banner(const Args &args, int threads, int32_t min_size, const std::string &output_path, uint64_t start_seed) { | ||
| std::printf("============================================================\n"); | ||
| std::printf("Minecraft Mushroom Island Search\n"); |
There was a problem hiding this comment.
This should probably say COMMISSION with a version number instead of the generic "Minecraft Mushroom Island Search" nobody refers to this program as.
| std::printf("unbound : %s\n", unbound ? "true" : "false"); | ||
| std::printf("print interval : %d\n", PRINT_INTERVAL); | ||
| std::printf("GPU devices : %s\n", join_devices(args.devices).c_str()); | ||
| std::printf("finding type : %s\n", current_finding_tag().c_str()); |
There was a problem hiding this comment.
This seems redundant if you're also displaying whether it's large biomes and unbound.
|
|
||
| std::string current_finding_tag() { | ||
| if (unbound) { | ||
| return large_biomes ? "ULB" : "USB"; |
There was a problem hiding this comment.
This isn't useful, and it's wrong because what if you're running with unbound but your result is in-bounds?
|
|
||
| void write_result_line(std::FILE *file, const SearchResult &result) { | ||
| const std::string tag = current_finding_tag(); | ||
| std::fprintf(file, "%s %" PRIi64 " %" PRIi32 " %" PRIi32 " %" PRIi32 "\n", |
There was a problem hiding this comment.
Same comment about the tag being wrong.
| } | ||
| std::printf("seed=%" PRIi64 " x=%" PRIi32 " z=%" PRIi32 " size=%" PRIi32 "\n", | ||
| r.seed, r.x, r.z, r.size); | ||
| std::printf(" Chunkbase : %s\n", make_chunkbase_url(r.seed, r.x, r.z, large_biomes).c_str()); |
There was a problem hiding this comment.
I'm not sure how I feel about baking in chunkbase/mcseedmap links for this. Maybe it should be a compiler option though if you did want to still have it.
| std::printf("Hello! large_biomes = %s\nunbound: %s\nprint interval: %d\n", large_biomes ? "true" : "false", unbound ? "true" : "false", PRINT_INTERVAL); | ||
|
|
||
|
|
||
| int32_t min_size = args.min_size.value_or(6'000'000 * (large_biomes ? 16 : 1)); |
There was a problem hiding this comment.
Why did you change the default from 10M (160M LB) to 6M (96M LB)?
| } | ||
|
|
||
| if (output_file != nullptr) { | ||
| std::fflush(output_file); |
There was a problem hiding this comment.
This might be the most "Correct" thing to do but since we're already closing the file anyway in the next line, I'm not sure it's necessary.
3 participants
Just changes to the console output mostly to make it a little nicer to look at.
One of the big QoL changes is that the output message for when you find a new seed will also contain a link to view it on either chunkbase or mcseedmap.