Pickups in map objects #478
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Refactored so just this change is in the PR now. |
cxong
requested changes
Feb 2, 2017
There was a problem hiding this comment.
Sorry for the delay in getting back to this, I've taken a closer look now.
The code that adds health/gun pickups should be in ObjRemove and inside an if (!gCampaign.IsClient) block, because:
- ObjDestroy purely destroys objects, so it would be inappropriate for it to do anything else. See how it's also called in a loop in ObjsTerminate, which happens during teardown - don't want to add pickups at the end of the game or anything like that!
- The game supports client/server multiplayer model, so need to be mindful of what things can happen independently or what things need to be server-authoritative. For example, if we independently spawn health pickups on each client, then we could have a health pickup that exists on one client but not another, so the client player can't pick it up because the server doesn't know about the health pickup. Instead only the server should handle things like spawning health pickups; it will then tell all the clients to add the health pickup at the same location so everything's consistent. Unfortunately the server and client code is pretty much the same, because this was hacked into an existing codebase which didn't support client/server model. The gCampaign.IsClient bool is how we can tell whether we are a client or a server. If you search the codebase for this, you can find it in a lot of places. It's very dirty, not architected properly, maybe one day we can redesign the code to make this more elegant... maybe.
src/cdogs/objs.c
Outdated
| void ObjDestroy(TObject *o) | ||
| { | ||
| // Random chance to add health pickup or gun pickup |
There was a problem hiding this comment.
This should be in ObjRemove, and wrapped inside if (!gCampaign.IsClient) block
There was a problem hiding this comment.
thanks for the feedback, I'm just getting my dev environment sorted out again after moving distros so I'll get this PR sorted out soon! 👍
|
Thanks! Good change |
cxong
added a commit
that referenced
this pull request
Feb 28, 2017
Add DestroySpawn array element to map_objects.json; allows spawning random pickups (Ammo, Health, Gun) at customisable random chance.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Did an implementation of "Pickups in destroyed map objects" in #367
But it does a gun drop with super low chance instead of an ammo drop.