Opened 8 years ago
Last modified 4 months ago
#4000 new defect
[WIP] Don't use BuildingAI for ships or siege tower
Reported by: | elexis | Owned by: | |
---|---|---|---|
Priority: | Must Have | Milestone: | Backlog |
Component: | Simulation | Keywords: | beta |
Cc: | b.jansen5@… | Patch: |
Description (last modified by )
Since ships were introduced they used BuildingAI
to shoot arrows when being garrisoned and use UnitAI
to have a stance, move around and get into attack range.
This resulted in inconsistencies like 1 arrow being shot from UnitAI
and N arrows being shot from BuildingAI
.
The code to determine the number of garrisoned arrows should be refactored so that both buildings and ships can call the same methods to shoot arrows without ships using BuildingAI logic to f.e. check for enemies in range.
Change History (22)
comment:1 by , 8 years ago
Summary: | Don't use BuildingAI for ships → Don't use BuildingAI for ships or siege tower |
---|
comment:2 by , 8 years ago
Cc: | added |
---|
comment:4 by , 8 years ago
Keywords: | rfc added |
---|---|
Milestone: | Backlog → Alpha 21 |
Summary: | Don't use BuildingAI for ships or siege tower → [WIP] Don't use BuildingAI for ships or siege tower |
Some WIP here: https://github.com/bb-bb/0ad/tree/t4000_buildingAI. This is just a draft for this ticket so don't kill me for style issues.
The approach in this WIP is to merge the attack logic from buildingAI with the ranged logic from attack and call those functions from either buildingAI or UnitAI. The buildingAI schema has got empty by these changes (the values moved to attack), So to keep the component living I putted some dummy in the schema, maybe there are cleaner ways to do this.
A side effect is that ships and siege towers no longer attack multiple enemy's but only 1, don't know if that is wanted (and this is a WIP after all...). Also they cannot move and shoot at the same time anymore. Allowing this again can be done with defining a new unitAI state like: WALKINGANDATTACKING. (That one can then also be used for e.g. chariots.)
This patch solves #4189 and makes ship ramming possible. Somewhat related #2010
comment:7 by , 8 years ago
For understanding the problem history, I add http://trac.wildfiregames.com/ticket/818#comment:5 See also r17784 and https://wildfiregames.com/forum/index.php?/topic/18003-suggestions-for-0-ad/&page=93 (and page 94)
comment:8 by , 7 years ago
Keywords: | beta added |
---|
comment:9 by , 7 years ago
Keywords: | rfc removed |
---|
comment:11 by , 7 years ago
Milestone: | Backlog → Alpha 22 |
---|
stanislas69: there has been some progress about that. There is now a plan about fixing that. I submitted it to bb and I hope we will have that for a22 (at least the first cleaning part). So I put it to alpha22 (not for the sake of bothering you ;-)).
comment:12 by , 7 years ago
I am really wondering what the "plan" is you talk about. We have had some discussion about implementing a new SiegeAI component, which I didn't see the benefit for as it would mostly be duplicating unitAI. Also with this we would create the same sort of situation between siegeAI and UnitAI as we have now between UnitAI and BuildingAI, and this ticket is about to solve the problem not just move the problem to some new component.
comment:13 by , 7 years ago
@bb: I was refering to a plan submitted in private section of the forum. The one we discussed in pm (not really what you described here, but I have certainly bad explained it ;-)).
edit: btw you did good posting that comment because you suggested a first idea of name SiegeAI
. That one is not really good as it applys to ship and stuff like that but it's a progress ;-)
comment:14 by , 7 years ago
Another possible way for this ticket would be that any garrisoned unit which currently gives an arrow would create a (invisible, indestructible and without obstruction) turret unit with a range attack. Ideally, this turret unit would be created using #252, but in the meantime for test purpose, this could work with a default turret template having only a range attack. That's just an idea i had some time ago but, as i did not really develop it more, there may still be some difficulties to deal with (the turrets should have the same los rangemanager as their parent; when the parent receives an attack order, this order should also be sent back to its turrets).
follow-up: 18 comment:15 by , 7 years ago
@fatherbushido: That's just a naming issue change SiegeAI
to TurretAI
or something similar.
@Mimo: in your approach we would end up with 1 unit shooting 1 arrow. How would we implement the arrow bonus tower tech? and arrow limit on that? ofc we can hack these things in but it's in concept not a very clean approach imo.
Maybe it would be better to use #2577 to solve this ticket, (I will post my draft for that ticket there). In this way we can have would have a unitAI unit as turretHolder and put a BuildingAI (or probably a turretAI?) unit as turret on that.
comment:16 by , 7 years ago
@bb: It was a positive comment.
There is also that mimo's idea to dig. But I guess anyway the step to split multiple arrows logic of buildingAI and of moving units is to be done anyway?
comment:17 by , 7 years ago
Indeed the multiple arrow logic from buildingAI needs to be splitted to somewhere else so moving units can use it too.
comment:18 by , 7 years ago
Replying to bb:
@fatherbushido: That's just a naming issue change
SiegeAI
toTurretAI
or something similar.@Mimo: in your approach we would end up with 1 unit shooting 1 arrow. How would we implement the arrow bonus tower tech? and arrow limit on that? ofc we can hack these things in but it's in concept not a very clean approach imo.
That would not be a problem. I don't know which particular tech you meant, but the one like Sentries would still apply as now (the turret stuff would only apply for garrison arrows) while a tech like Crenellations would decrease the repeattime by 40% of such turret units.
The original interest of my proposition was that turrets are dealt with by UnitAI, so don't need a new component. But such a new component could indeed be cleaner, depending how it is implemented.
Maybe it would be better to use #2577 to solve this ticket, (I will post my draft for that ticket there). In this way we can have would have a unitAI unit as turretHolder and put a BuildingAI (or probably a turretAI?) unit as turret on that.
comment:19 by , 7 years ago
Milestone: | Alpha 22 → Backlog |
---|
comment:20 by , 5 years ago
Component: | UI & Simulation → Simulation |
---|
Move tickets to Simulation
as UI & Simulation
got some sub components.
comment:21 by , 3 years ago
Description: | modified (diff) |
---|
This is probably a dumb comment, but I have to say it... Why should ships use unit AI, building AI, or turret AI? Can't they have ship AI? Ships are not a type of unit, nor a type of building, nor a type of turret. They are perhaps something inbetween, similar to a siege unit AI perhaps; but not the same thing, anyhow. And yes, ramming ships should be possible; and therefore a ship should be able to sink from a hull breach, and not only from having its "health" brought down to zero. All the more reason for them being their own thing.
I don't open a new ticket as it's kinda related: Siege Tower capture attack doesn't work. When targetting a building, we show the capture attack, then the siege tower stay idle near the building. This is due to the priority of the buildingAI range attack on the siege tower capture attack. Removing BuildingAI from the siege tower solve this issue (in this case, the tower capture building or fire is arrow as every range unit).