omf2097 / openomf

One Must Fall 2097 Remake
http://www.openomf.org
MIT License
362 stars 35 forks source link

Snaf AI rework #387

Closed Snafulator closed 2 years ago

Snafulator commented 3 years ago

AI is a bit more fun to play against now but still a bit too timid for my liking at the higher difficulties.

AI Rework:

HAR object changes:

katajakasa commented 3 years ago

Thanks for the PR! It might be that your line endings don't match the original files (CRLF vs CR vs LF). Some editors like to reformat the whole file when edited, and git has the whole autocrlf thing too. I'd really like to have proper diffs to be able to review :) I will take a proper look at this during the weekend.

Snafulator commented 3 years ago

Just did a rebase and fixed the formatting issues. You should be able to see the changes properly now :)

katajakasa commented 3 years ago

Okay, finally had some time to test this. Currently the code does not compile. GCC 10.3.0 hits the following errors:

`Tuomas Virtanen@Lyra MINGW64 ~/omf/openomf/build
$ make
Consolidate compiler generated dependencies of target openomf_core
[  1%] Building C object CMakeFiles/openomf_core.dir/src/controller/ai_controller.c.obj
C:/msys64/home/tuomas/omf/openomf/src/controller/ai_controller.c: In function 'assign_move_by_cat':
C:/msys64/home/tuomas/omf/openomf/src/controller/ai_controller.c:570:16: error: too many arguments to function 'is_valid_move'
  570 |             if(is_valid_move(move, h, a->pilot)) {
      |                ^~~~~~~~~~~~~
C:/msys64/home/tuomas/omf/openomf/src/controller/ai_controller.c:493:5: note: declared here
  493 | int is_valid_move(af_move *move, har *h) {
      |     ^~~~~~~~~~~~~
C:/msys64/home/tuomas/omf/openomf/src/controller/ai_controller.c: In function 'assign_move_by_id':
C:/msys64/home/tuomas/omf/openomf/src/controller/ai_controller.c:594:16: error: too many arguments to function 'is_valid_move'
  594 |             if(is_valid_move(move, h, a->pilot)) {
      |                ^~~~~~~~~~~~~
C:/msys64/home/tuomas/omf/openomf/src/controller/ai_controller.c:493:5: note: declared here
  493 | int is_valid_move(af_move *move, har *h) {
      |     ^~~~~~~~~~~~~
C:/msys64/home/tuomas/omf/openomf/src/controller/ai_controller.c: In function 'attempt_attack':
C:/msys64/home/tuomas/omf/openomf/src/controller/ai_controller.c:759:20: error: too many arguments to function 'is_valid_move'
  759 |                 if(is_valid_move(move, h, a->pilot)) {
      |                    ^~~~~~~~~~~~~
C:/msys64/home/tuomas/omf/openomf/src/controller/ai_controller.c:493:5: note: declared here
  493 | int is_valid_move(af_move *move, har *h) {
      |     ^~~~~~~~~~~~~
make[2]: *** [CMakeFiles/openomf_core.dir/build.make:272: CMakeFiles/openomf_core.dir/src/controller/ai_controller.c.obj] Error 1
make[1]: *** [CMakeFiles/Makefile2:143: CMakeFiles/openomf_core.dir/all] Error 2
make: *** [Makefile:156: all] Error 2
katajakasa commented 3 years ago

I short description on the principles behind the AI "thinking process" on top of the aio_controller.c file would be great (a separate markdown document works too). That way future devs (and reviewers :) ) can more easily figure out what is going on.

katajakasa commented 3 years ago

I wrote down some things, will continue later when I have more time again. Real Life has the tendency to interfere :(

Snafulator commented 3 years ago

Okay, finally had some time to test this. Currently the code does not compile. GCC 10.3.0 hits the following errors:

`Tuomas Virtanen@Lyra MINGW64 ~/omf/openomf/build
$ make
Consolidate compiler generated dependencies of target openomf_core
[  1%] Building C object CMakeFiles/openomf_core.dir/src/controller/ai_controller.c.obj
C:/msys64/home/tuomas/omf/openomf/src/controller/ai_controller.c: In function 'assign_move_by_cat':
C:/msys64/home/tuomas/omf/openomf/src/controller/ai_controller.c:570:16: error: too many arguments to function 'is_valid_move'
  570 |             if(is_valid_move(move, h, a->pilot)) {
      |                ^~~~~~~~~~~~~
C:/msys64/home/tuomas/omf/openomf/src/controller/ai_controller.c:493:5: note: declared here
  493 | int is_valid_move(af_move *move, har *h) {
      |     ^~~~~~~~~~~~~
C:/msys64/home/tuomas/omf/openomf/src/controller/ai_controller.c: In function 'assign_move_by_id':
C:/msys64/home/tuomas/omf/openomf/src/controller/ai_controller.c:594:16: error: too many arguments to function 'is_valid_move'
  594 |             if(is_valid_move(move, h, a->pilot)) {
      |                ^~~~~~~~~~~~~
C:/msys64/home/tuomas/omf/openomf/src/controller/ai_controller.c:493:5: note: declared here
  493 | int is_valid_move(af_move *move, har *h) {
      |     ^~~~~~~~~~~~~
C:/msys64/home/tuomas/omf/openomf/src/controller/ai_controller.c: In function 'attempt_attack':
C:/msys64/home/tuomas/omf/openomf/src/controller/ai_controller.c:759:20: error: too many arguments to function 'is_valid_move'
  759 |                 if(is_valid_move(move, h, a->pilot)) {
      |                    ^~~~~~~~~~~~~
C:/msys64/home/tuomas/omf/openomf/src/controller/ai_controller.c:493:5: note: declared here
  493 | int is_valid_move(af_move *move, har *h) {
      |     ^~~~~~~~~~~~~
make[2]: *** [CMakeFiles/openomf_core.dir/build.make:272: CMakeFiles/openomf_core.dir/src/controller/ai_controller.c.obj] Error 1
make[1]: *** [CMakeFiles/Makefile2:143: CMakeFiles/openomf_core.dir/all] Error 2
make: *** [Makefile:156: all] Error 2

Addressed in commit: https://github.com/omf2097/openomf/pull/387/commits/d112d33b19cfab04e952dba5ff2fc8783dc73ff0

katajakasa commented 3 years ago

Okay. this compiles for me now, and seems to work. I will probably share the binary to others to test too, and I'll get back to this in a few days. I went through the code more thoroughly, and added a few more things that should/could be improved. Other than that, I couldn't find anything to complain about. So far so good :)

Out of curiosity, do you have plans on handling those things on your TODO list ?

Snafulator commented 3 years ago

Okay. this compiles for me now, and seems to work. I will probably share the binary to others to test too, and I'll get back to this in a few days. I went through the code more thoroughly, and added a few more things that should/could be improved. Other than that, I couldn't find anything to complain about. So far so good :)

Out of curiosity, do you have plans on handling those things on your TODO list ?

There is one i plan on doing in the short-term: Making the SPAM tactic throw basic attacks instead of repeating the last move (or atleast adding some more variation).

Other than that i have noted a few bugs in this PR which i will log as issues.

Another couple of ideas i want to look into are:

joneirik commented 3 years ago

Some feedback after playing through 1player 4 times, champion, deadly, ultimate difficulty.

The main problem with the AI as is now, is that it really doesn't Attack, it just randomly throws out some attacks at random time. It needs some sort of logic to try to guide it to make more efficient attacks.

Probably make a basic AI for all bots that tries to do a few things like for ex:

Once that is done, make variants per bot, where you can specify more which moves works for what etc. Then Tweak that to work with the pilot unique stats.

Defensively the AI is already better than the original-AI, largely for not having the old loopholes, and at Deadly/Ultimate to blocks nearly anything instantly. Champion felt a bit better with leaving some openings.

It still has a few new loopholes though, and I realised I could beat most of the AI with 6p, 4k, and Throw on jaguar (Tried this on Champion and Deadly). Similarly it's love to attack the moment it jumps means that the AI is largely punishable on falling down from jumps. Amusingly this makes the AI an absolute killer against anyone that is used to fighting against the Original-AI, and can't adjust to not just using jump attacks for entry. Also since most AI doesn't attack until REALLY close, you can usually just let them walk into you and throw them.

I don't know how you use the unique-character-stats with this, I noticed some small difference between the pilots, but mostly it meant that some characters like Cossette almost didn't attack at all. I'm not a code monkey so I can't read up and understand what you did, but it felt like you set the AI at 0% chance to start with, and added the Unique-Character-Stats on top of that, which likely would have given Cossette something like 5% chance to attack or something (It's low, depending on which stats you're looking at). If that is the case, might be worth it to put the base % to something like 50% before the Unique-character-stats, and see how that works.

(I find Github confusing as heck, so I'm easier to get hold of on the discord btw)

katajakasa commented 2 years ago

@Snafulator Hey, so this is looking pretty good, and seems to work better than our current AI. Should we just merge, or wait for more changes ?

Snafulator commented 2 years ago

@Snafulator Hey, so this is looking pretty good, and seems to work better than our current AI. Should we just merge, or wait for more changes ?

If you are happy enough with the code quality then we can merge it in. I have no functional changes that i want to make at the moment.

I need to stop rebasing and squashing commits as it has made your review comments hard to track against the current code.

katajakasa commented 2 years ago

Aeh, I somehow missed the couple still seemingly open code review comments. I'll leave this open for now until you (or someone) has the time to hammer those down :) It's not like we are in a hurry :D