Removes Crit Chance preproc

[AlexOn1ine]

Introduces the function GetCritHitOdds to switch between generational critical hits.

Also I turned the macro BENEFITS_FROM_LEEK into an inline function for better readability and removed ASSUME(B_CRIT_CHANCE >= GEN_7); from crit_chance.c. Correct me if I'm wrong but it seems to me that it isn't needed by any current tests. If I'm wrong I can add it back to the specific tests which would need it.

[AlexOn1ine]

added assumes back where needed

[Pawkkie]

Looks great Alex, the only comment I had on initial skim in the car this morning was on the ASSUMEs, I think a few got missed but this seems pretty much ready to go to me :)

[AlexOn1ine]

I'll take care of everything tomorrow. I'm not sure about the Chansey thing but wouldn't mind changing it. I only really did the change because the macro was so long (and I dislike macros) but I think it is fair to make the change for Chansey as well.

[Pawkkie]

I'll take care of everything tomorrow. I'm not sure about the Chansey thing but wouldn't mind changing it. I only really did the change because the macro was so long (and I dislike macros) but I think it is fair to make the change for Chansey as well.

Completely agree on the macro -> function front I find them much more readable and as a result much easier for users to change themselves :)

[AlexOn1ine]

Ready for review.

I've made a singular function for both leek and lucky punch. Should be easy to extend without duplicating to much code. Hope that's a fine solution.

[AlexOn1ine]

did some more refactoring but for some reason the test Crit Chance: Signature items Leek and Lucky Punch increase the critical hit ratio by 2 stages started failing for seemingly no reason.

I will continue work on this tomorrow but I wouldn't mind feedback for the current state. I personally think it is much better now since all users will have to do now is edit the GetHoldEffectCritChanceIncrease to make a new leek type affected mon

[AlexOn1ine]

force push because I made constant changes that changed one var

[Pawkkie]

I really like the GetHoldEffectCritChanceIncrease change! Just think we need to either use it for all the gen 1 items or none of them.

If you got that test sorted, I think this is the last feedback I'll have :)

[Pawkkie]

If we want to handle Gen 1 Leek with this function, I think we should also handle Gen 1 Lucky Punch and Scope Lens, have similar defines for them, and use this function in the Gen 1 version also.

Or we don't use this function in the gen 1 version at all, if you don't want to have 3 defines here. I don't like the idea of using this function for some gen 1 held items but not all of them.

[AlexOn1ine]

fair point! will work out a solution tomorrow

[AlexOn1ine]

If you got that test sorted, I think this is the last feedback I'll have :)

I was not able to find the problem because the recording was fine.

[Pawkkie]

I was not able to find the problem because the recording was fine.

Oh I just assumed you had it sorted because it doesn't fail with the CI. That is very strange. Would you say that's blocking in your opinion? It's odd to me that it doesn't fail here but does on your machine.

[AlexOn1ine]

I was not able to find the problem because the recording was fine.

Oh I just assumed you had it sorted because it doesn't fail with the CI. That is very strange. Would you say that's blocking in your opinion? It's odd to me that it doesn't fail here but does on your machine.

I added a KNOWN_FAILING and yes it is blocking imo