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

Re-add vehicles again #247

Conversation

Capital-Asterisk
Copy link
Contributor

@Capital-Asterisk Capital-Asterisk commented Oct 29, 2023

This 100% finishes the pipeline rewrite and fixes everything commented-out in #244

  • Update all the old task functions to the new Pipeline one
  • Re-Add Signal Links (wiring). It just works and is far more elegant than what was previously there due to the Pipeline nested loop feature
  • Re-Add Machines: Rockets, RCS drivers, User Controls
  • Add 'Thrust Indicators' to indicate which rockets are firing. This required adding 'Draw Transform Observers'
  • Split code into different files

DrawTransformObservers

from drawing_fn.h:

/**
 * @brief Function pointers called when new draw transforms are calculated
 *
 * Draw transforms (Matrix4) are calculated by traversing the Scene graph (tree of ActiveEnts).
 * These matrices are not always stored in memory since they're slightly expensive. By default,
 * they are only saved for DrawEnts associated with an ActiveEnt in ACtxSceneRender::activeToDraw.
 *
 * Draw transforms can be calculated by SysRender::update_draw_transforms, or potentially by a
 * future system that takes physics engine interpolation or animations into account.
 * DrawTfObservers provides a way to tap into this procedure to call custom functions for other
 * systems.
 *
 * To use, write into DrawTfObservers::observers[i]
 * Enable per-DrawEnt by setting ACtxSceneRender::drawTfObserverEnable[drawEnt] bit [i]
 *
 */
struct DrawTfObservers

Other notes

One may notice that in many places I'm dropping the m_ naming convention for structs that don't have (or have very few) member functions. The m_ is useful for member functions as C++'s implicit this-> can make it difficult to tell which variables are or aren't member variables. If there are no member functions, then they're redundant and contribute to making my eyes bleed as I try to understand what the code is doing, so they should be removed.

@jonesmz
Copy link
Member

jonesmz commented Oct 30, 2023

Well that's an enormous diff.

I reviewed it as best i could and didn't see anything glaring. But no promises that i didn't overlook something.

@jonesmz
Copy link
Member

jonesmz commented Oct 30, 2023

if you rebase on master, everything in the CI should work except for the windows analyzer.

@jonesmz
Copy link
Member

jonesmz commented Oct 30, 2023

Ok now CI is 100% green.

@jonesmz
Copy link
Member

jonesmz commented Oct 30, 2023

You can see the code scanning results here: https://github.com/TheOpenSpaceProgram/osp-magnum/security/code-scanning

@Capital-Asterisk
Copy link
Contributor Author

Thanks for the CI fixes!

@Capital-Asterisk Capital-Asterisk marked this pull request as ready for review October 30, 2023 23:48
@Capital-Asterisk Capital-Asterisk merged commit a65640f into TheOpenSpaceProgram:master Oct 31, 2023
16 of 22 checks passed
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.

2 participants