Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Truckload of updates #96

Merged
merged 64 commits into from
Mar 2, 2022
Merged

Conversation

madebr
Copy link
Collaborator

@madebr madebr commented Feb 27, 2022

Highlights:

  • blip in the map is working:
    Screenshot from 2022-02-27 22-43-12
  • save + load screens + logic "working".
    I don't guarantee any compatibility with the original carmageddon savegames.
    So create a backup before using it, is advised...
    Screenshot from 2022-02-28 00-32-28
  • APO keys working (and not working in single player campaign):
    Screenshot from 2022-02-28 00-37-57
  • Various implementations
Overall progress:
55% of all functions implemented. (1900 completed / 3471 total)

@dethrace-labs
Copy link
Owner

dethrace-labs commented Feb 28, 2022

While this is awesome work, as usual(!), this kind of PR is a little difficult for me to handle.

PRs that implement a feature or behavior end-to-end are easier to
a) test before merging
b) because we can test them, that ensures all the code going in to main doesn't just compile, but actually executes correctly (as best as we can tell).

This PR has some things that should be easy to merge:

  • Load / Save full end to end
  • Partial Map rendering (we have enough to see that what you've done so far is working)

Things that I don't mind as extras in the PR:

  • Extra BRender functions that might/might not be required specifically for dethrace
  • Tidying (replacing 0's with NULL, adding STUB calls where appropriate, removing incorrectly inlined function calls etc etc

Things that make it hard to merge:

  • Functions implemented which either aren't called yet, or are part of a feature that is not completed end-to-end, so I have no way to sanity check except by looking at each function myself:
  • Examples: StripBlendedFaces, the crush code, the sparks code, the shrapnel code, flame code, smoke code, etc.
  • MakeCarIt is a good example of what I'm trying to say - that code is not going to be used until we have network play. When we start on network play, that would be a good time to implement it, because it will get used and tested. Right now it has an implementation which we don't know is right or not. As we add functionality, I want to be able to trust the code that is already there. It makes debugging so much nicer when we don't hit bugs in a function that was implemented earlier but never validated.

I'm torn, because I see all your good work, but I want to get into the habit of merging full features without so much random extras

if (i < 0 || pTrack_spec->ampersand_digits <= i) {
return 1;
}
r1 = BR_SQR3(pActor->t.t.mat.m[0][0], pActor->t.t.mat.m[0][1], pActor->t.t.mat.m[0][2]);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I normally use BrVector3LengthSquared here - probably BR_SQR3 could be removed from the public brender include

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used this one because I found it weird to cast a row of a matrix to a vector, but it makes sense to do it.
So I'll do that from now on.

@dethrace-labs
Copy link
Owner

Going to merge this now

@dethrace-labs dethrace-labs merged commit 6568a15 into dethrace-labs:main Mar 2, 2022
@madebr madebr deleted the here-we-go-again branch March 2, 2022 06:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants