slipher commented Jan 19, 2022

@bmorel I've moved the most common and trivial renaming parts into a separate commit (the 2nd one). The 1st commit now consists of the original fir…

slipher commented Jan 19, 2022

Yeah, part of my objection is the semantics are unclear from the name. But also it returns a different result from the original code, in the case o…

slipher commented Jan 19, 2022

Huh?

slipher commented Jan 19, 2022

pm should be global either everywhere or nowhere.

slipher commented Jan 19, 2022

This is not a good thing to make a member function. pml_t is a hodgepodge of variables which doesn't correspond to any coherent concept, so it is n…

slipher commented Jan 19, 2022

Function names should begin with a capital letter

slipher commented Jan 19, 2022

setFireBeam got into PM_ClimbMove PM_WalkMove dedup (to PmoveSingle) somehow You don't have to fix it if you don't want to

slipher commented Jan 19, 2022

What's the meaning of this? I think "Slide" is just talking about that 1 if block.

slipher commented Jan 18, 2022

@necessarily-equal @ing in case you want to investigate the reverted one

bmorel commented Jan 18, 2022

Concerning the revert, it appears that it's the reason why packages does not work at all, even with custom maprotation file. I do know know the rea…

bmorel commented Jan 18, 2022

Well, I'm fine with not merging it, or maybe only merge some. Namely: fix shift-sign-overflow fix array-bound fix virtual classes lacking virtual …

slipher commented Jan 18, 2022

Hopefully the various teams using Crunch would be able to converge on a maintained fork at some point. I fear that mass reformatting commits would …

bmorel commented Jan 18, 2022

I see. Thanks for explanations.

bmorel commented Jan 18, 2022

The 1st commit I believe just contains the trivial find-and-replace stuff like Rocket -> Rml and CString -> c_str. Nope, there's more than just t…

slipher commented Jan 18, 2022

You can check out the rmlui3/sync branch on Daemon to get C++14 activated. The 1st commit I believe just contains the trivial find-and-replace stuf…

slipher commented Jan 18, 2022

It doesn't work with predictedPlayerState - it's all zero the first time this executes

slipher commented Jan 18, 2022

Yeah that's what I did on my inactive ASAN branch. slipher@237ff1b

bmorel commented Jan 18, 2022

I do not know how CI passes, but here I can't compile at all, something unsets the -std=c++14 that I put in the CXXFLAGS, somewhere, somehow, and s…

bmorel commented Jan 18, 2022

I checked what CG_MyTeam() does, and it checks cg.predictedPlayerState.persistant[PERS_TEAM]. Why the change? (I don't say this is wrong, I focused…

bmorel commented Jan 18, 2022

Actually, there's an even better solution: to move the code which uses those parameters out of the function, since it's only used by BG_PlayerState…

bmorel commented Jan 18, 2022

I believe all comments have been addressed properly now, except for «This commit (PM_ContinueLegsAnim PM_StartLegsAnim dedup ) makes the code harde…

necessarily-equal commented Jan 18, 2022

This makes me think that for ASAN we could simply encapsulate the whole block like this: #if !defined(__has_feature) || !__has_feature(address_sani…

bmorel commented Jan 18, 2022

LGTM. I don't know the relations with NaCl, so like necessarily-equal I'll trust you on that point.

bmorel commented Jan 18, 2022

makes me thinking about 3 (not related to C++14) things: there is no new[] / delete[] operators builds don't pass when using USE_ADDRESS_SANITIZER …

necessarily-equal commented Jan 18, 2022

I'll trust you on that one

bmorel commented Jan 18, 2022

Will do that then.

bmorel commented Jan 18, 2022

Well, I had explanations since then about why crn is better than webp (uses less GPU memory, for reference), so I guess such a cmake could use crn …

slipher commented Jan 18, 2022

Currently there are a couple of options: Use Urcheon for a fancy build with crn compression, delta paks and so on Idiot method - copy nexe's into …

slipher commented Jan 18, 2022

Please squash last commit :)

slipher commented Jan 18, 2022

How about instead of bool extrapolate, int time to have Util::optional<int> extrapolateFromTime? Then there aren't two bool arguments (confusing), …

slipher commented Jan 18, 2022

Personally I find it easier to follow when the original commit that had mistakes or improvements is modified to include the changes, rather than ha…

Gireen commented Jan 17, 2022

or using a script to place the buildables. That's indeed a serious problem. The other problems are existing ones but the change clearly could mak…

bmorel commented Jan 17, 2022

Ah, well, yes didn't noticed the *0, I was focused on the 0x12346.... silly me

slipher commented Jan 17, 2022

Crashing of course :)

bmorel commented Jan 17, 2022

Fun fact: there is no such entry.

bmorel commented Jan 17, 2022

@slipher : I addressed the comments, but to keep things easier to track, since have not merged them, instead I pushed more commits on top. I intend…

bmorel commented Jan 17, 2022

I suppose doing that (returning a ref instead of a pointer) should be addressed in a different PR (as this one is enormous enough as it is, and thi…

slipher commented Jan 16, 2022

Also update the ckit option description to mention repairing?

bmorel commented Jan 16, 2022

From what I've seen, generating navmeshes accounting for the jump's height poses lot of problems on parpax (but again, with current navmesh generat…

bmorel commented Jan 16, 2022

Note that the screens here are on station15

slipher commented Jan 16, 2022

Moving the code that sets point around won't make the code more efficient, and this is the part that is hard to read, so I'd rather have this chang…

bmorel commented Jan 16, 2022

Weird.

slipher commented Jan 16, 2022

Apparently this warning can't trigger without -O2. All of our builds are currently in Debug mode, so it can't be triggered.

bmorel commented Jan 16, 2022

By applying #1691 without last commit on master. Gcc version 10.2.1 from debian.

slipher commented Jan 16, 2022

How do you even trigger this warning? I tried a bunch of different code samples and GCC versions and couldn't get it on any of them.

bmorel commented Jan 16, 2022

This breaks the GUI, for some reason.

bmorel commented Jan 16, 2022

maybe document what's this line is while you're at it?

bmorel commented Jan 16, 2022

I agree with slipher: the only reason I erroneously used Math::Clamp() without reading return value was because this is what unvanquished's API usu…

bmorel commented Jan 16, 2022

Understandable, but considering this is done a lot, I wanted to reduce the number of calculations. This commit have zero impact on the API though, …

bmorel commented Jan 16, 2022

true enough, will change that. It's just kind of an habit I've taken while reading bot code :)

bmorel commented Jan 16, 2022

good catch

bmorel commented Jan 16, 2022

exact.

bmorel commented Jan 16, 2022

When I get a pointer (and not a reference) from a function, I always consider things can go wrong and thus always put an assert after if I do not h…

bmorel commented Jan 16, 2022

Good catch, needs an additional "else" block there

bmorel commented Jan 16, 2022

Thanks, will update the comment.

bmorel commented Jan 16, 2022

I think you'll change your mind after some commits, since code surrounding PM_ContinueLegsAnim and PM_StartLegsAnim gets moved to be organised in a…

bmorel commented Jan 16, 2022

So this requires navgen changes? Yes and no. Of the 3 changes, only the crouching requires navmesh change. The lesser-autojump brings improvement…

slipher commented Jan 16, 2022

This one PM_ClimbMove PM_WalkMove dedup (to PmoveSingle) is not correct because there were early returns from PM_WalkMove, but now the rest of the …

slipher commented Jan 16, 2022

Useless assertion as BG_Class always returns something even if given an out-of-range value.

slipher commented Jan 16, 2022

Nit: our style for variable names is wpAnim

slipher commented Jan 16, 2022

In a .cpp file this should be static not inline

slipher commented Jan 16, 2022

pm->ps is only 6 characters. There doesn't need to be a variable for it.

slipher commented Jan 16, 2022

Commit PM_CorrectAllSolid cleanup definitely makes the code harder to read.

slipher commented Jan 16, 2022

This commit (PM_ContinueLegsAnim PM_StartLegsAnim dedup ) makes the code harder to read IMO

slipher commented Jan 16, 2022

Looks like 1 is toes in water, 3 is "eyes" in water (viewheight), and 2 is halfway between toes and eyes.

slipher commented Jan 16, 2022

So this requires navgen changes? In that case the navgen changes should be in the same PR, as navgen now lives in the same repository. Navgen chang…

slipher commented Jan 16, 2022

I would rather keep a = Clamp(a,b,c). I think ClampArg just makes it harder to read. Other commits look good.

slipher commented Jan 16, 2022

LGTM

slipher commented Jan 16, 2022

I've been using TODO(0.53)

bmorel commented Jan 15, 2022

Actual testing seems to show that the code works fine. About navmeshes, though, there is new data. To generate navmeshes which takes the most out o…