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

Ai.c improvements #517

Merged
merged 19 commits into from
Sep 15, 2015
Merged

Ai.c improvements #517

merged 19 commits into from
Sep 15, 2015

Conversation

ids1024
Copy link
Member

@ids1024 ids1024 commented Aug 24, 2015

This seems to me like a good first step for improving the ai module. Needless to say, it is not the only improvement to be made.

@ids1024
Copy link
Member Author

ids1024 commented Aug 25, 2015

Come to think of it, do not merge this yet. I will keep pushing changes to further improve ai.c.

@ids1024 ids1024 changed the title Make ai code deal with pilot objects instead of ids [WIP] Ai.c improvements Aug 25, 2015
@bobbens
Copy link
Member

bobbens commented Aug 25, 2015

For legacy reasons the AI uses a lot of custom stuff. I always wanted to incorporate the standard Lua modules we have into it. I mean there is a nlua_pilot.c and stuff which should be able to work directly in the AI. This would also allow doing coolre things.

@ids1024
Copy link
Member Author

ids1024 commented Aug 25, 2015

@bobbens Yep, I'm working now on removing the duplicate functions and having the ai code use the standard ones.

@ids1024
Copy link
Member Author

ids1024 commented Aug 29, 2015

I'm not quite sure which functions should be kept and which removed. For example, ai.dist(target) could be replaced by something like ai.getPilot():pos():dist(target.pos()), but ai.dist() is used in quite a few places and that is a bit long and I'm sure at least somewhat less efficient.

It is similar with the ai.rel*() functions.

@ids1024 ids1024 changed the title [WIP] Ai.c improvements Ai.c improvements Aug 30, 2015
@ids1024
Copy link
Member Author

ids1024 commented Aug 30, 2015

Ok, I think this is in somewhat good shape now, though perhaps some testing is needed to make sure everything is working. I have removed the WIP from the title.

Use forward declaration of struct to deal with cyclical import.
@ids1024
Copy link
Member Author

ids1024 commented Sep 5, 2015

@bobbens @Deiz @BTAxis How do the changes here seem?

@bobbens
Copy link
Member

bobbens commented Sep 15, 2015

Haven't looked into it in too much detail, but it looks good. Some of the nice shortcuts have been removed and that makes you use chained commands, but I think it's perfectly mergeable as is. Cheers,

@bobbens bobbens merged commit 4d51f9e into naev:master Sep 15, 2015
@ids1024 ids1024 deleted the ai branch September 15, 2015 22:21
@ids1024
Copy link
Member Author

ids1024 commented Sep 16, 2015

@bobbens A few shortcuts have been removed, but I think it is worth it for the api consistency. I left in shortcuts like dist and relvel which could be done in lua, but would be somewhat complicated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants