-
Notifications
You must be signed in to change notification settings - Fork 183
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
[Ready] Garages #480
[Ready] Garages #480
Conversation
rwengine/src/data/ModelData.hpp
Outdated
@@ -249,6 +249,44 @@ class SimpleModelInfo : public BaseModelInfo { | |||
return related_; | |||
} | |||
|
|||
static bool isDoorModel(std::string m) { | |||
if (m == "oddjgaragdoor") return true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As a way to separate the data here, the list of names can be std::(unordered_)set.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will do
rwengine/src/engine/GameWorld.cpp
Outdated
@@ -76,6 +76,10 @@ GameWorld::~GameWorld() { | |||
for (auto& p : allObjects) { | |||
delete p; | |||
} | |||
|
|||
for (auto& p : garageControllers) { | |||
delete p; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Storing each controller in a unique_ptr would give you this for free.
rwengine/src/engine/GameWorld.cpp
Outdated
o->getModelInfo<BaseModelInfo>()->name)) | ||
continue; | ||
|
||
garageMidpoint.x = (coord0.x + coord1.x) / 2; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't need to be calculated inside the loop
@@ -45,6 +45,13 @@ InstanceObject::InstanceObject(GameWorld* engine, const glm::vec3& pos, | |||
engine->aigraph.createPathNodes(position, rot, path); | |||
} | |||
} | |||
|
|||
// @todo this is temporary, remove when physics gets fixed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What needs to be fixed about the doors?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is related to #450, I want to replace this code later with setStatic
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resolved
rwengine/src/data/ModelData.hpp
Outdated
@@ -114,6 +115,20 @@ class BaseModelInfo { | |||
using ModelInfoTable = | |||
std::unordered_map<ModelID, std::unique_ptr<BaseModelInfo>>; | |||
|
|||
static std::unordered_set<std::string> doorModels = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One could think it would be const by default, as changing an unordered_set is not allowed (to my understanding). Weird.
rwengine/src/engine/GameWorld.cpp
Outdated
// Create controller | ||
std::unique_ptr<GarageController> garageController( | ||
new GarageController(this, info, door)); | ||
garageControllers.push_back(std::move(garageController)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe it's better to create when "inserting". I think about push_back(std::make_unique(...))
.
return false; | ||
} | ||
|
||
case GarageType::BombShop1: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you should add in "empty cases" break;
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not empty, it falls through
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So it is intentional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bombshops and resprays logic is different though, so this is incorrect anyway. But I was advised not to add more logic as there is a lot of code already.
#107
This PR todo list:
Garages todo overall: