Skip to content

Add GameData methodmap#766

Merged
Headline merged 1 commit intoalliedmodders:masterfrom
peace-maker:gameconf_methodmap
Oct 12, 2018
Merged

Add GameData methodmap#766
Headline merged 1 commit intoalliedmodders:masterfrom
peace-maker:gameconf_methodmap

Conversation

@peace-maker
Copy link
Member

@peace-maker peace-maker commented Feb 5, 2018

Wrap gamedata natives in a pretty methodmap and update stock plugins/includes to use it.

This fixes bad documentation on GameConfGetAddress and not closing the gameconfig file if handle creation fails as well.

@asherkin
Copy link
Member

asherkin commented Feb 6, 2018

I'm not a fan of the name GameConf, but don't really have a better suggestion, opening up to the floor? @alliedmodders/sourcemod

Implementation looks sounds, nice catch on the fixes.

@peace-maker
Copy link
Member Author

peace-maker commented Feb 6, 2018

Could call it GameData since that's what people use to refer to it.

@Fyren
Copy link
Contributor

Fyren commented Feb 6, 2018

I think with the existing natives named the way they are, I'd prefer GameConf.

@Headline
Copy link
Member

Headline commented Feb 7, 2018

Personally, I think that naming it GameData is a better choice even though it differs from the original native's naming scheme. It looks cleaner, and this is an evolving language anyway. 😛

I just wanted to give my two cents, if that's welcome.

@peace-maker
Add GameData methodmap
5728795 
Wrap gamedata natives in a pretty methodmap.

This fixes bad documentation on `GameConfGetAddress` and not closing the gameconfig file if handle creation fails as well.
@peace-maker peace-maker changed the title Add GameConf methodmap Add GameData methodmap May 26, 2018
@psychonic
Copy link
Member

psychonic commented Jul 8, 2018

I have no strong feelings one way or the other on the naming. Everything else looks okay to me.

Headline
Headline approved these changes Jul 31, 2018
View reviewed changes
Copy link
Member

@Headline Headline left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this lgtm; much cleaner and is a good addition to the mm api. 🥂

@Headline Headline merged commit 1b795a7 into alliedmodders:master Oct 12, 2018
@peace-maker peace-maker deleted the gameconf_methodmap branch October 12, 2018 07:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Headline Headline Headline approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@peace-maker @asherkin @Fyren @Headline @psychonic