View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0000553 | Spring engine | General | public | 2007-06-24 21:59 | 2007-09-11 12:45 |
| Reporter | KDR_11k | Assigned To | tvo | ||
| Priority | normal | Severity | feature | Reproducibility | N/A |
| Status | resolved | Resolution | fixed | ||
| Summary | 0000553: [patch] AimFromWeapon, fixedLauncher and other tweaks | ||||
| Description | - Weapons now use AimFromWeapon to determine the part to aim from, this prevents waggling and failing to aim at close ranges. Internally this means weaponMuzzlePos holds the position where projectiles get spawned, not weaponPos. - Melee weapon now reports a proper angle to AimWeapon. - Added fixedLauncher tag (bool) for missiles and starburst missiles, this makes the projectile spawn with the orientation of the shooting piece instead of their normal orientation. The weapon will not properly check if allies are in the way so make sure you align the launchers in a way that won't hammer right into your own forces and perhaps use collideFriendly=0. FixedLauncher conflicts with trajectoryHeight and IMO combining them is pointless anyway so don't use both on a weapon. - Made starburst missiles obey startvelocity and weaponacceleration, if your mod set them to something silly because they didn't do anything before you'll have to adjust. - Added projectiles tag (int), each time a weapon fires this many projectiles are spawned (on the same frame). Make sure you put them on different trajectories somehow (sprayangle or different firepoints) because otherwise they'll all be clumped in one shot. Can be set to 0 as well for script trigger weapons and can be combined with burst. - Added COB call to "ShotX" that happens shortly before weaponX uses QueryWeaponX, use this for switching firepoints in bursts or when using multiple projectiles. - Missiles and starburst missiles now obey smoketrail, set it to 0 to disable the smoke. To get a missile without smoketrail you have to define the weapon as a missile with WeaponType=MissileLauncher in the tdf. - StartBuilding now supplies the pitch as the second parameter to help with custom build FX. | ||||
| Additional Information | Since I lack experience with Spring coding it would be helpful if a more experienced dev looked through the changes to see if there are any mistakes or style errors. The patch was taken against revision 3842. | ||||
| Tags | No tags attached. | ||||
| Attached Files | |||||
| Checked infolog.txt for Errors | |||||
|
|
Index: rts/Sim/Units/UnitTypes/Builder.cpp =================================================================== --- rts/Sim/Units/UnitTypes/Builder.cpp (revision 3886) +++ rts/Sim/Units/UnitTypes/Builder.cpp (working copy) @@ -583,9 +583,12 @@ { float3 wantedDir=(pos-this->pos).Normalize(); short int h=GetHeadingFromVector(wantedDir.x,wantedDir.z); + short int p=(short int) (asin(wantedDir.dot(updir))*(32768/PI)); + short int pitch=(short int) (asin(frontdir.dot(float3(0,1,0)))*(32768/PI)); std::vector<int> args; args.push_back(short(h-heading)); + args.push_back(short(p-pitch)); cob->Call("StartBuilding", args); int soundIdx = unitDef->sounds.build.getRandomIdx(); Why is p calculated with updir and pitch with float3(0,1,0)? Any (mathematical) reasons for that or a bug? In at least MissileProjectile.cpp, after applying your patch, code like this results. Spaces are marked with underscores, other indentation are tabs. ____if (weaponDef->visuals.smokeTrail) if(drawTrail){ //draw the trail as a single quad } else { //draw the trail as particles } Now how does this look with 4 character tabs: if (weaponDef->visuals.smokeTrail) if(drawTrail){ //draw the trail as a single quad } else { //draw the trail as particles } Please don't do this, it makes the code really hard to read, especially since the pieces I left out in the inner if are very big chunks of code. Indent the big chunk properly so it is clear it is a nested if block, and so it doesn't look totally borked on anything but 8 character tabs. The same, applies to the lines like: if (weaponDef->visuals.smokeTrail) SAFE_NEW CSmokeTrailProjectile(pos,oldSmoke,dir,oldDir,owner,false,true,7,Smoke_Time,0.6f,drawTrail); (all on one line) These lines are really too long, put the SAFE_NEW ... part on the next line. For the rest it looks about right, though I should look somewhat better at the bigger changes. Could you fix/comment the above points? |
|
|
I'm not sure what happened with the indentations but I suppose it's the fault of my editor, context. If you press enter at the wrong point it creates the indents with spaces instead of tabs. Not sure about the updir/0,1,0 issue either, I think I copied the pitch calculation from the weapon code. |
|
|
But do you see my point? You added an extra top-level if condition just by indenting it less/differently from the if below it, instead of actually indenting the if below it one level more. I wouldn't really care if it was a 5 line if condition without else, but this is one huge if with an else block, so writing an if before it makes it unclear what the 'real' indentation depth of the inner block is, and it makes it unclear to which if the else belongs. |
|
|
Oh, THAT. I probably forgot to indent the block... I do use size 4 tabs, maybe I didn't indent it because I wasn't sure I was disabling the right block of code and wanted to do that after I'm sure it works. I'll add a file with the mentioned things fixed |
|
|
BTW, please don't commit it right now, I'm making some further additions and I don't think I could make a separate patch with those easily. |
|
|
please commit the part responsible for turning Melee type weapons towards their targets, currently they turn to the west no matter where the target is. |
|
|
I really have no idea how I make patches of a part of the changes. I could upload my current patch state but I've made some weapons able to fire and travel underwater if waterweapon=1 is set but it doesn't apply to all weapons and I'm not sure if I can leave it at such an incomplete change. |
|
|
Okay, I've done my additions. The new changelog looks like this: - Weapons now use AimFromWeapon to determine the part to aim from, this prevents waggling and failing to aim at close ranges. Internally this means weaponMuzzlePos holds the position where projectiles get spawned, not weaponPos. To simulate the old behaviour make AimFromWeapon return the same piece as QueryWeapon does. - Melee weapon now reports a proper angle to AimWeapon. - Added fixedLauncher tag (bool) for missiles, starburst missiles and torpedoes, this makes the projectile spawn with the orientation of the shooting piece instead of their normal orientation. The weapon will not properly check if allies are in the way so make sure you align the launchers in a way that won't hammer right into your own forces and perhaps use collideFriendly=0. FixedLauncher conflicts with trajectoryHeight and IMO combining them is pointless anyway so don't use both on a weapon. - Made starburst missiles obey startvelocity and weaponacceleration, if your mod set them to something silly because they didn't do anything before you'll have to adjust. - Added projectiles tag (int), each time a weapon fires this many projectiles are spawned (on the same frame). Make sure you put them on different trajectories somehow (sprayangle or different firepoints) because otherwise they'll all be clumped in one shot. Can be set to 0 as well for script trigger weapons and can be combined with burst. - Added COB call to "ShotX" that happens shortly before weaponX uses QueryWeaponX, use this for switching firepoints in bursts or when using multiple projectiles. - Missiles and starburst missiles now obey smoketrail, set it to 0 to disable the smoke. To get a missile without smoketrail you have to define the weapon as a missile with WeaponType=MissileLauncher in the tdf. - StartBuilding now supplies the pitch as the second parameter to help with custom build FX. - Missiles, starburst missiles and torpedoes now use flighttime if present instead of the hardcoded formula (which is still used if flighttime is not present) - Added SubMissile tag (bool) for TorpedoLaunchers, if set to 1 the torpedo will travel like a missile outside of water. It will not emit a smoketrail, though. When fired from a plane it will behave the same way but the plane will probably be considered a fighter, not a bomber. - All weapons (save for the Rifle which noone uses anyway) can now attack underwater targets if they have waterweapon=1 set. - Added fireSubmersed tag (bool), defaults to the value of waterweapon. If set the weapon can fire underwater, if not then not. Use it for weapons that cannot fire underwater but can hit underwater targets. Works even for torpedoes. - Starburst missiles now obey turnrate for the turn at the peak of their flight (if supplied, otherwise the old value is used). - Bombs can use manually defined burst and burstrate values instead of autogenerated ones by using the new manualBombSettings tag (bool). Keep in mind that (roughly) burst*burstrate defines the area in which the bomber is willing to drop the bomb so if you make it too small the bomber might miss the drop area and not drop any bomb. - Added myGravity tag (float) for ballistic weapons (Cannon and Bomb), it overrides the map gravity if used. Regular map gravity is around 0.2. - Bombs now obey Accuracy and SprayAngle. - Fixed lightning weapons having a hardcoded inaccuracy, made it obey accuracy and sprayangle tag instead. - Fixed Lightning weapons not doing damage to shields. - Added ShieldStartingPower tag (float) for shields, if set the shield starts with this much power instead of 0. I occassionally got a crash the moment I hit enter when the "waiting for connections, press Enter to start" message was visible but I have no idea how to translate the stacktrace in a custom build so I couldn't track it down. It didn't happen consistently and only very rarely. No idea if that was my patch or something in the SVN version. I think the patch was taken against rev 4014 or so. |
|
|
kdr: you need to $ scons configure debug=yes optimize=yes if you're building with mingw/gcc. start the debugger $ gdb spring (gdb) run and when it crashes (gdb) bt this is true both in linux and windows. note that optimize=yes might give strange backtraces, but optimize=no might not cause any crashes. |
|
|
Committed fixes to bombs and lighting cannon. Too afraid to commit the rest ;) I'd like someone else to take a look or two. Oh, committed the melee fix sometime earlier. |
|
|
I noticed that the SVN version now has a changed range limiter system for starbursts, this should be disabled if fixedLauncher or Flighttime are used for the weapon since then the range measurement is either off (it doesn't check for range use during the "ascent" which isn't vertical with fixedLauncher) or limiting (for a guided missile you wouldn't want the range limit to override the mod-defined flighttime so the missile can keep tracking the target even if it moves quickly) |
|
|
imbaczek: can you make a patch of the remaining patch so I can take a look? |
|
|
will do. |
|
|
uploaded a patch that compiles. starburst missiles are broken but don't have the time to fix them now. will try later, for now don't commit them. |
|
|
don't remember what I changed, but from my limited testing nothing's broken. kdr, if you have some test mod using those features, please test if everything works as advertised. |
|
|
conflicts against r4223 resolved. |
|
|
Tobi, have you taken a look at the patch? I'm tempted to commit it by the end of the week, so it gets some testing by mod devs. Been running with it for 3 weeks now and didn't notice any wrong behaviour, so it doesn't break anything that I test with (mostly BA, but a little bit of Gundam, too, and the recent bug-exposing CvC.) |
|
|
No, didn't really get around to it yet. Skimmed through it once, but that doesn't really count :-) It is OK if you commit it especially if you have given it so much testing already. I will look at it later then and fix anything I may see then. |
|
|
ok, committed in r4264. may the search for bugs commence. |
|
|
Ok, looked through it in a bit more detail and didn't really see anything obviously wrong so I think this can be closed ? If not, just reopen :) |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2007-06-24 21:59 | KDR_11k | New Issue | |
| 2007-06-24 21:59 | KDR_11k | File Added: weaponChanges.patch | |
| 2007-06-25 10:13 | tvo | Summary | AimFromWeapon, fixedLauncher and other tweaks => [patch] AimFromWeapon, fixedLauncher and other tweaks |
| 2007-07-10 23:17 | tvo | Note Added: 0000995 | |
| 2007-07-12 08:29 | KDR_11k | Note Added: 0000997 | |
| 2007-07-12 15:25 | tvo | Note Added: 0000998 | |
| 2007-07-12 18:11 | KDR_11k | Note Added: 0000999 | |
| 2007-07-12 18:12 | KDR_11k | File Added: weaponChangesFix1.patch | |
| 2007-07-18 20:32 | KDR_11k | Note Added: 0001014 | |
| 2007-07-19 19:23 | imbaczek | Note Added: 0001018 | |
| 2007-07-19 22:36 | KDR_11k | Note Added: 0001022 | |
| 2007-07-19 22:37 | KDR_11k | Note Edited: 0001022 | |
| 2007-07-20 17:12 | KDR_11k | File Added: evenMoreWeaponChanges.patch | |
| 2007-07-20 17:16 | KDR_11k | Note Added: 0001024 | |
| 2007-07-23 10:58 | imbaczek | Note Added: 0001029 | |
| 2007-08-11 10:38 | imbaczek | Note Added: 0001078 | |
| 2007-08-17 09:56 | KDR_11k | Note Added: 0001111 | |
| 2007-08-18 11:45 | tvo | Note Added: 0001121 | |
| 2007-08-18 12:17 | imbaczek | Note Added: 0001122 | |
| 2007-08-18 14:16 | imbaczek | File Added: kdr11k_weapon_changes_r4220.patch | |
| 2007-08-18 14:17 | imbaczek | Note Added: 0001125 | |
| 2007-08-18 17:05 | imbaczek | File Added: kdr11k_weapon_changes_r4220_v2.patch | |
| 2007-08-18 17:07 | imbaczek | Note Added: 0001126 | |
| 2007-08-18 22:41 | imbaczek | File Added: kdr11k_weapon_changes_r4223_v3.patch | |
| 2007-08-18 22:41 | imbaczek | Note Added: 0001127 | |
| 2007-08-29 20:12 | imbaczek | Note Added: 0001189 | |
| 2007-08-30 20:15 | tvo | Note Added: 0001192 | |
| 2007-08-30 23:16 | imbaczek | Note Added: 0001193 | |
| 2007-09-11 12:45 | tvo | Status | new => resolved |
| 2007-09-11 12:45 | tvo | Resolution | open => fixed |
| 2007-09-11 12:45 | tvo | Assigned To | => tvo |
| 2007-09-11 12:45 | tvo | Note Added: 0001242 |
