Closed yibeili closed 6 months ago
ccjeremylo
commented
6 months ago Also, I guess we need to add configs for this change? I don't think this has been done?
Are we going to default to MVV_LVA or KILLER_MOVE? I suspect this discussion is related to #114
KYLChiu
commented
6 months ago Also, I guess we need to add configs for this change? I don't think this has been done?
Are we going to default to
MVV_LVAorKILLER_MOVE? I suspect this discussion is related to #114
Which configs need to be added?
For the default, let's keep it as is for now. The naive idea is that usually captures (MVV_LVA) are more valuable than quiet (killer).
The use case for killer moves will be clear when we refactor the move ordering - we'll have a composite move ordering class/methodology which combines MVV_LVA, Killer and future move ordering evaluations.
Btw, if you're wondering why there isn't a check for captures when adding killer moves - it isn't clear to me whether this should be done in the composite class level at the moment or at when adding killer moves in searcher. I suggested to delay until we do the refactor.
Some general comments 🤔 - usually it is nice to put some description for the PR:
This is for documentation purposes and also helps reviewer to get some context - of course there might be some duplication with the original issue, e.g. #4.
People usually do it if the PR is long-ish and/or not immediately obvious what the change is about. Sometimes if a PR is very short, e.g. 3 lines of code change, then of course no one does it.