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

Merged QuakeWorld qc Code into codebase. #163

Merged
merged 22 commits into from
Sep 30, 2024
Merged

Conversation

Zungrysoft
Copy link
Collaborator

@Zungrysoft Zungrysoft commented Sep 17, 2024

Merged the NetQuake and QuakeWorld codebases into a single codebase so any changes we make will support both engine types. We will now be able to distribute a qwprogs.dat for QW deathmatch play.

You can compile the codebase for either NetQuake (creating progs.dat) or QuakeWorld (creating qwprogs.dat). You can compile for QuakeWorld by passing the argument -D__QW__ into fteqcc. Otherwise, it will compile for NetQuake.

Wherever there is a discrepancy between the NQ and QW implementations, here is how I resolved it:

  • If it only affects singleplayer/coop, use the NQ implementation.
  • If it only affects deathmatch, use the QW implementation.
  • If it affects both:
    • If the difference is required due to differences in communication with the engine or available engine features, branch the code using preprocessor directives.
    • Otherwise, branch the code based on whether deathmatch is currently enabled.

Or in short, singleplayer behaves the same as it did before, but deathmatch now always behaves like QuakeWorld, regardless of which engine type is being used.

QW Source used for this merge: https://github.com/id-Software/Quake/tree/master/qw-qc

I'm still working on testing this, though it's ready for review.

@Zungrysoft Zungrysoft marked this pull request as ready for review September 18, 2024 06:05
Copy link
Collaborator

@MotoLegacy MotoLegacy left a comment

Choose a reason for hiding this comment

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

Thanks for your work on this, left you some feedback and questions.

@lavenderdotpet
Copy link
Owner

what changes are currently holding this back

@Zungrysoft
Copy link
Collaborator Author

@lavenderdotpet

The only things I have left to do is determine whether qwprogs.dat should end up in pak0.pak or be unpacked and test a few more things related to deathmatch. Then @MotoLegacy needs to re-review it and answer my followup questions.

@MotoLegacy
Copy link
Collaborator

Hi @Zungrysoft, I don't see any follow up questions that you asked for me to answer.

@Zungrysoft
Copy link
Collaborator Author

@MotoLegacy Actually, I think that's it. Just need your re-review. Thanks!

Copy link
Collaborator

@MotoLegacy MotoLegacy left a comment

Choose a reason for hiding this comment

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

Thanks for your work on this! Looks good to me.

@Zungrysoft
Copy link
Collaborator Author

@MotoLegacy Great! Thanks! Do I need a second review? Who else in the community has touched the progs.dat code?

@lavenderdotpet
Copy link
Owner

lavenderdotpet commented Sep 30, 2024

Thanks for your work on this! Looks good to me.

should i aprove this i dont really know what im looking at when it comes to code but between u and zungry yall are like the main 2 active people who work in code maybe random brushes should review this and not me?

edit: not to imply randombrushes is inactive i just remembered he worked on the code mid typing the sentence

@MotoLegacy
Copy link
Collaborator

Yes, I will request a review from @RandomBrushes

Copy link
Collaborator

@RandomBrushes RandomBrushes left a comment

Choose a reason for hiding this comment

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

Only have one minor comment for client.qc, but that's a stylistic thing regarding nesting if/elseif/else and shall not block merging.

}
else if (deathmatch)
SetNewParms ();
Copy link
Collaborator

Choose a reason for hiding this comment

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

It appears that the if-body and the else-if-body are doing the same (SetNewParms()). This most likely can be rephrased, but that not functionally important (doesn't block merge, IMO).

@lavenderdotpet lavenderdotpet merged commit e9cb95a into main Sep 30, 2024
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.

4 participants