-
Notifications
You must be signed in to change notification settings - Fork 183
Big'N'Veiny pickups #493
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
Big'N'Veiny pickups #493
Conversation
This has not gotten any review the past 2+ weeks. Can you provide more information or even a screenshot to attract reviewers? I assume this can be tested by starting the http://gta.wikia.com/wiki/Big%27n%27Veiny mission. Some possible things you could have included in the first post:
|
@@ -81,6 +85,34 @@ class PickupObject : public GameObject { | |||
return false; | |||
} | |||
|
|||
virtual bool isBigNVeinyPickup() const { | |||
return false; | |||
} |
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.
nit: This does look slightly out of place.
Does GTA3 really do it this way? I'd have expected some kind of enum to identify the pickups by type, instead of such a very specific getter which is only useful for one single item type, used in a single mission of the game.
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.
GTAIII does this very poorly, there is separate global pool for these pickups, and separate update, render functions, and special creation and removal functions for opcodes.
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.
There is no, like, actual pickup type, which describes what this pickup is, in original game. We already have behavior specific type variable [same as in original game]. GTAIII just uses model index for that. For example if behavior type is PICKUP_ONCE or PICKUP_ONCE_TIMEOUT, etc it checks for model if it's health pickup model then give health. I guess we can add type enum, and ditch all these special classes maybe?
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 fine with me then (for now).
I guess we could have something like a PickupPool
struct in the future which could be more similar to what GTA3 does.
But I feel this is such a small portion of the project that it's not worth time to redesign anything at this point. The solution is ugly, but it dos work.
I guess you can squash the "Minor fixes" commit into the first commit, or give the commit a better title which is more descriptive. Theoretically most commits could be named "Minor fixes" or "Major fixes" - so avoid such generic names. I've skimmed over the code and it looks ok. I did not test it yet and I'm waiting for more information until I feel confident to give my final approval. |
It's almost playable, mission softlocks in the end, due to missing AI.
Yes
If you are talking about only Big'N'Veiny pickups then I don't think so.
There is no special behaviour, except they only can be picked up in vehicle.
Will do |
rwgame/RWGame.cpp
Outdated
@@ -507,10 +507,13 @@ void RWGame::tick(float dt) { | |||
clockAccumulator -= 1.f; | |||
} | |||
|
|||
constexpr float timerClockRate = 1.f / 25.f; |
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 don't fully understand this change yet (I'm also not aware of the surrounding code).
Where does this / 25.f
come from - gta3.exe?
Does it have to do with FPS or what's the story behind it?
also nit for this commit: commit title is too long; should be reworded to not break into commit description
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.
Does it have to do with FPS or what's the story behind it?
Basically, yes, it's tied to FPS.
In GTAIII timers are updated every frame and framerate is locked to 30fps [if you have frame limiter on], there is no accumulators or anything. They may go out of sync depending on game's uptime, due to double precision.
bool onPlayerTouch() override; | ||
}; | ||
|
||
const static std::array<glm::vec3, 106> bigNVeinyPickupsLocations = { |
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.
nit: Can it be constexpr? It should be doable on almost every distro. (GLM 0.9.8.0 - 2016-09-11)
LGTM |
Fixed: mission timer Timer was going crazy due to missing 02d9 opcode, update timer 25 times per second Fixed: mission timer Don't beep on every timer update
Uh oh!
There was an error while loading. Please reload this page.