MSVC compiler errors

[ghost]

(this got the library working on visual studio 2015 for me)
Compiler Error C2440
Compiler Error C2664
Compiler Warning (level 4) C4703

[krychu]

Hey @ggsg-francis ! Thanks for the PR.

I like having a custom rand. But was wondering what's your reasoning? Is it to not use standard rand, performance?

[ghost]

Oh I'm sorry, I didn't mean to submit that change, only the edits that solved my compiler errors. I added the custom rand for myself because I'm using your library to generate maps in an RTS game and I wanted to be sure of its determinism.

[krychu]

I think this is a good change. I'd like to keep it. Determinism is useful so that you can generate the same results from the same seeds. And also it might be useful that we don't alter state of rand(), in case the user app uses it for something else.

The only thing that I'd suggest is that we pass seed as part of wfc_run instead of wfc_init, because wfc_init doesn't have to be called before the first run. Currently wfc_run takes two arguments: the wfc struct and max_collapse_cnt: https://github.com/krychu/wfc/blob/master/wfc.h#L1120

I think the latter is not really that useful, we should change it to seed. -1 would indicate to use a random seed (e.g., time(NULL)), and non-negative values would be used as the actual seeds. How does this sound to you?

An alternative would be to introduce wfc_seed(seed) which could be called between the runs, but this seems more convoluted?

I'm not familiar with SquirrelNoise3, need to read about it. Looks very performant which is always very welcome, does it have some other good properties why you use it?

Thanks again!

[krychu]

I'm also thinking this could be a #define switch to opt into custom rand implementation, wdyt?

[ghost]

Hi, I'm glad you like my changes :> I've removed those constexprs now

The reason seed is passed to wfc_init (and wfc_overlapping) in my version is because in the original library, wfc_init is where srand is called. I don't want to pass the seed into wfc_run, I want to set the seed once and run the generation multiple times for different purposes. So if you really want to change it, I would say using wfc_seed(seed) is the most versatile, and it would also be backwards compatible with the existing API, which is nice.

I'm also thinking this could be a #define switch to opt into custom rand implementation, wdyt?

Could do, I avoid putting code behind ifdefs where possible but it's your library and it doesn't bother me.

I didn't think too hard about the noise function. it's just the one I happened to know about, and the author says it's got a very even distribution, which is good enough for me.

By the way, is including float.h necessary for any reason?

[krychu]

float.h is for DBL_MAX only, I created an issue to remove it: #26

[ghost]

I see, but having the seed in wfc_init is not good then, because you have to call wfc_init before each run (if you're re-using the same struct wfc) and so you are forced to pass the seed for each run rather than once at the beginning. Is this right? or am I missing something?

Nah, you're right, I want to change it now. I looked at my code, and I use wfc_overlapping each time. how cool.

The only thing I'm slightly concerned about is that the specific rand implementation could have big impact on the quality of results. I'll do some testing and comparisons. If we switch from std rand to squirrel or something else, we should be sure it's not leading to worse results.

Okay, let me know!

[krychu]

I created issue to work on the selection of the better noise(): #27

In this talk: https://www.youtube.com/watch?v=LWFzPP8ZbdU I found that the implementation is a bit different from yours:

your impl:

Notably in the reference implementation the second noise is added rather than multiplied, and the second bit shift is to the left << rather than to the right >>.

Is this something you were experimenting with?

I also noticed there was some issue reported with this noise and author since improved the implementation. I'll paste more info in the issue.

[ghost]

Is this something you were experimenting with?

no, that's just a mistake.............................................................

[krychu]

no worries, good to know. We're on a good track though. Next week I'll probably finish testing strongest candidates including squirrel 3 and 5. This has been very helpful thanks.

[krychu]

The lib is C99. Can we change constexpr to const or #define?

[ghost]

Sorry for my impulsive programming

[krychu]

all good, good stuff coming through in this PR. Let's figure out good noise :) We can merge this PR faster if you remove noise implementation for now before I do a bit more research and testing.