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

Changed oter_t from using bools to usng bitset. #11446

Merged
merged 1 commit into from Mar 21, 2015

Conversation

Projects
None yet
5 participants
@Angular-Angel
Copy link
Contributor

commented Mar 2, 2015

This is just the changes to switch over to the bitset. I'm not entirely sure I did the whole Pull Request propery, so you should probably take a close look at it. It's prety short, so it shouldn't be too much trouble.

@Coolthulhu

This comment has been minimized.

Copy link
Contributor

commented Mar 2, 2015

Looks OK to me.
One thing: I don't like that default argument on set_flag. I'd make it explicit in all cases.

@@ -22,6 +22,27 @@ struct overmap_spawns {
int max_population;
int chance;
};
//terrain flags enum! this is for tracking the indices of each flag.
//is_asphalt, is_building, is_subway, is_sewer, is_ants,

This comment has been minimized.

Copy link
@BevapDin

BevapDin Mar 3, 2015

Contributor

Why did you repeat the enum values here?

This comment has been minimized.

Copy link
@Angular-Angel

Angular-Angel Mar 3, 2015

Author Contributor

To make them easier to find for people reading through the code. I'll probabl remove the default argument from the set_flag function, but I'll leave that until the next pull request.

This comment has been minimized.

Copy link
@BevapDin

BevapDin Mar 3, 2015

Contributor

They are listed again right below it as part of the enum definition, why repeat them?

If one can read them in the comment, one can also read them in the actual code. Do you really think someone sees them in the comment but is unable to find them in the actual definition? And I usually use the search-in-document function (or similar) of the editor when I need to find some definition.

This comment has been minimized.

Copy link
@KA101

KA101 Mar 3, 2015

Contributor

Agreed that in this context the comment-listing is unhelpful. Something along the lines of "add new terrain flags to this list" (presuming that that is in fact the case) would likely be more helpful.

This comment has been minimized.

Copy link
@Angular-Angel

Angular-Angel Mar 4, 2015

Author Contributor

Oh, that's because I wrote the comment before I wrote the Enum, and never rewrote the comment.

@kevingranade kevingranade merged commit 166fe2f into CleverRaven:master Mar 21, 2015

1 check failed

default Unmergeable pull request.

@Angular-Angel Angular-Angel deleted the Angular-Angel:WorldgenRevisions branch Mar 21, 2015

@Angular-Angel

This comment has been minimized.

Copy link
Contributor Author

commented Mar 21, 2015

Oh, cool. I had forgotten to go back and update it for compatibility with 0.C, but it looks like you took care of that. Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.