ryndrb / dota2bot

DotA 2 Bot Script: Tinkering ABo(u)t
19 stars 5 forks source link

[Suggestion] Code clean and improve code quality #31

Closed dmknght closed 3 months ago

dmknght commented 6 months ago

This bot script is based on Beginner AI script, which based on Furious Puppy and other scripts, so It has some issues IMO:

  1. The code structure is kinda basic. For example: we are having 3 LUA scripts of mode push ub 1 folder, and the same for 3 scripts mode defend.
  2. Code syntax doesn't follow any standard. For example, 1 condition code block could follow the 1 linear syntax, the other follows multiple lines syntax.
  3. Unknown code block, lack of comments. Like in this line: https://github.com/ryndrb/dota2bot/blob/master/mode_rune_generic.lua#L24, or the other code blocks which fresh contributors don't know the code flow, which event triggers current code blocks, ....

So I'd suggest:

  1. Change structure of code following tree folders
  2. Improve syntax in every scripts
  3. Add as many comments as possible.
forest0xia commented 6 months ago

why don't you do it? His time should better spend on improving the bot's ability instead of code quality. if you care it so much, make it satisfy you and us. Im happy to review your code.

ryndrb commented 5 months ago

[scribble] Related to current Spell Refactoring... init.

forest0xia commented 5 months ago

This is not really related to the spell refactoring. And please don't spend your time on improving code structure at all. People play your script not because the code look nice. But because you have more working bots. Player does not need to look at the code at all. And you don't get more players because the code look nicer but because the bots look nicer. Please spend your time more on the bots logical thinking, decision making, instead of improving the code or refactoring the files.

forest0xia commented 5 months ago

Software engineers with different levels of understanding of the code and business will have different opinions on what it means to be a well structured code. And we do not need a great code structure at all. Unless your future bot IQ enhancement work is blocked due to the fact the code structure is broken then please refactor. But otherwise I can point out almost every single line in this code base can be improved with coding style, data structure, inheritance, oop, computational complexity, memory/cpu usage optimizations, etc. Coding style or structure is just a minor piece of this script and should be consider as a P2 item on priority, where P0 is more bot supports or existing bot performance like ganking, etc.

forest0xia commented 5 months ago

For example the refactoring you are doing right now doesn't make sense to me because you just moving the code from 1 file to multiple files and they are all individual files for each hero, so originally you only need to update a single source of truth place for the hero specific configs, now you need to go to different places, find the file, fine the lines to do the modification - this then makes all the files in /BotLib/ folder meaningless since now inside the files in that folder, its just a bunch of variable reassignments or unused variables at all, you dont need to keep those files anymore by keep doing the refactoring, you can move those reassignment to other generic files for all heroes since the code are basically the same for all - but why do you keep refactoring without actually improving the bots abilities.

The spell refactoring can start with all in one file and then you can divide and improve the structure if you have time, for making things happen sooner - better in a generic module, where you keep the table {ability_name -> ability_function}, and the generic distance, nearby units, enemies, allys, etc calculations on top, and pass it down to ability functions for usage decision making.

forest0xia commented 4 months ago

You must have your own ideas and you can keep doing whatever you like. I'm just trying to clarify about the priority about this kind of scripts in general from the perspective of a general player that:

  1. we care the most about the playful of the bots, aka difficulty of the bots.
  2. then the available number of bots in the script, aka diversity.
  3. then efficiency of the script, aka smoothness, higher fps.
  4. code quality is the least on the priority. no one need to care about it and coders should figure out by themselves what does each line means and accept the world is never going to be perfect for them, please fix the world in the way you like by yourself.

Please spend your time wisely so more players can get benefits from your incredible work of improving the bot script playability.

forest0xia commented 4 months ago

In fact, chatgpt can do the code quality job better than anyone. unify the style, simplify the structure, add comments to all lines, translating comments/notes from one language to another language for coders who does not know how to read English comments lol.

ryndrb commented 4 months ago

I do understand what you're saying.

Player count is not something I care about as this is something I tinker with for my own enjoyment really. I originally uploaded it to the workshop mainly to ease bug fixing lol since it's easier to spot issues with more eyes on it. People using/liking it and providing helpful feedbacks are always great and appreciated, of course.

I did consider a single-file module/structure (similar to item usage.. or maybe you're referring to something else), but it will obviously become too long. It won't really impact performance, but it just makes it annoying to load. X.SkillsComplement etc,. inside BotLib can be removed since I can just call a key-value pair inside AbilityUsageThink directly. So that folder can just be dedicated to item/ability/talent build. Don't think having generic calls matters much, or at all, performance wise in this case from what I've noticed, since each spell have different ranges etc and feel more effective if curated. It's more of a Local Host thing, for the most part.

And code quality/structure is not something I care about in this script lol. Just figured it was relevant to this issue. The whole purpose of this is for Rubick (and probably Morphling). Splitting them in different files hardly matters at all; the code base is pretty small; and I just like having each in a different file; and it works this way for Rubick since I don't like him having generic spell usage. Besides, editing spell functionality is not really something I do all the time at this point, since most of them mostly works fine etc,. But regardless of what to use, it's just tedious.

Idk tho, might just resort to generic usage for Rubick.

forest0xia commented 4 months ago

That sounds good @ryndrb . My only concern was you spend too much time on refactoring by moving code around and not actually improving the performance of the bots or bug fixes. Player count do matter in many ways and at the very least more players can give you more helps on bug capturing & reporting as well as improvement suggestions. The code base is small so it does not really matter if we put a lot things in a file or split them into pieces. The general strategy tho is to keep similar stuff or things fall into the same strategy piece in one place like farming, pushing, etc those are shared logic, but it's fine in the implt details as it's more personal preference as well and they are all good as long as the approach is effective. Each spell does have different range etc, but in general there are fair amount of similar repeating calculations e.g. check if surrounded by X number o enemies in 1000 then don't use etc, those are redundant calculations and at most need to be counted only once per hero per frame.

We have the same point about code quality and it's low priority stuff. And the refactoring you are doing for Rubick is not the refactoring this post is asking, if I understood it correctly. That person posting this question was basically asking if you can improve the code in a way he can read what's going on in the code better, but you are not really simplifying the structure directly, or adding comments or improve syntax, at least for now, that's why I was confused and concerned if you'd spend more time on those soon. If you want to accomplish both at the same time to improve code style or quality and improve Rubick/morphling, I just wanted to suggest a priority but I appreciate your work in any way you like to go. Thanks