Alternative implementation for seed search (#120)

[moulins]

This is an alternative implementation for the seed search fonctionality.
I have implemented (roughly) the features I proposed in this comment.

I have taken a completely different approach than @jmessenger235 and @mgod, so I had to scrap almost all of their code, but I think it's worth it.

As a result, it doesn't have (yet) all the features of jmessenger235's last pull request (#169).

It should also be a little faster, but, I didn't do any benchmarks, so maybe I'm completely wrong (it did seem faster on the few tests I made).

Todo list (in rough order of priority) :

• Search inside arbitrary circular or square region
• Mutiple & nested criteria
• Specify search via .json file
• Structure criteria
• Optional criteria, maybe with a score system
• Some form of named criteria
• Better error reporting
• Multithreaded search
• Better comments in source code

[jmessenger235]

What is the use case that makes this strategy for json structure and code architecture worth the re-implementation? (Of both the features already tested and are part of my other PR and the lengthy list of ones that I have drafted but not committed as part of other testing I'm doing) I ask as I don't see the benefits and see some notable downsides to certain redundancies in the json and notably increased testing challenges. As such, I would like to better understand your perspective. By arbitrary region do you mean areas not centered around 0,0 and of non 512 divisible lengths?

[moulins]

Ok, so I'll start by explaining briefly how it works.
The filter is separated in two parts :

• the type Constraint represent a constraint on a world (currently there is only ConstraintBiome but there will be other types, structures for example). They are either satisfied or not yet satisfied.
• the type Criterion represent a criterion as written in the .json. They can be nested one inside another (with CriterionAnd and CriterionOr), and they are either satisfied, not satisfied or unknown. Each criterion manages its constraints, and evaluate only those which are needed. (for example, CriterionOr doesn't need to evaluate all its criteria if one of them is already satisfied).

I wanted also to address a limitation of mgod's implementation : the biome data is computed all at once, which can cause memory issues when searching a large area. To resolve this, I've broken up the search area in fragment-sized chunks, and all Constraints are tested on each chunk in one pass.
As soon as the global criterion's result is determined (either satisfied or not satisfied), we break and return.

This method enables us to use areas of arbitrary size (non divisible by 512) and positions (not centered on (0,0)), as they will be broken up into fragments before checking.
Also, multithreading should be relatively easy : just check the constraints on several fragments at once and only check the global criterion periodically.

In the end, I think this implementation is more flexible, and should have somewhat better performances characteristics.

---

I ask as I don't see the benefits and see some notable downsides to certain redundancies in the json and notably increased testing challenges.

I don't understand very well what you mean by "certain redundancies in the json", could you clarify ?
As for the testing difficulty, I think it will be inevitable if we want to support somewhat complex queries ; but testing several queries on several seeds and verifying the result should catch the majority of errors.

[stefandollase]

First of all, again, I appreciate the interest in #120 and your effort in helping to implement it. I did not yet look into it, but I agree with @jmessenger235: Why do you reimplement a feature that is already implemented and in the process of being integrated into the mainline? You claim that your implementation might be quicker, but at the same time you are not sure about it.

The appropriate course of action would have been to describe your findings (like possible problems) in #120 and to wait for my changes that help to better integrate the current implementation into the existing Amidst code base. After these are done, it is the right time to discuss the concrete query syntax and possible optimizations for the search algorithm. Providing a complete alternative implementation of the feature certainly does not speed up the process.

This is not a competition about who can provide the best implementation. Please let's collaborate on finding and solving the issues, so we end up with a single good implementation. Writing code is the easiest part of software development. Identifying the requirements and solving the issues is hard. While your implementation might fulfil certain requirements and solve several issues, it is hard to spot these in the implementation. Also, it is easier to discuss and agree on changes than it is to cleanly merge different implementations. Thus, let's have a high-level discussion about the changes in #120, before we implement any changes in code. Thanks.

[jmessenger235]

@stefandollase I agree, lets get your changes in and in the mean time have some further discussion about what is needed.

@moulins let's move this discussion to #120 per the request. I will add my comments there.

[jmessenger235]

It would also be useful if we could add a file selection windows to the search window so we aren't limited to having to constantly edit search.json when you are messing around with completely different searches.

[moulins]

I'm closing this PR: I'm still interested by writing a seed search implementation for amidst, but the code in this branch is being heavily refactored/modified to take into account all the discussions we've had and the WIP specification, and doesn't work anymore.

I will reopen another PR when I have a working prototype for the new semantics.