TheSil / base_enhanced

sil's base_enhanced server mod
3 stars 4 forks source link

Siege classes with CFL_SINGLE_ROCKET are able to obtain more rockets #95

Closed deathsythe47 closed 9 years ago

deathsythe47 commented 9 years ago

There is a classflag CFL_SINGLE_ROCKET that gives your guy a one-time-use rocket. Kind of a cool idea for a class to have one chance with a rocket.

CFL_SINGLE_ROCKET,//has only 1 rocket to use with rocketlauncher

However, this functionality wasn't finished by Raven, because even though you only spawn with one rocket, you are still able to obtain more rockets (through ammo dispensers, allied techs, canisters, etc) up to the maximum of 25! You shouldn't be able to obtain ANY more rockets. You should only have one shot, period.

You can see in the comments that Raven intended to prevent players from obtaining more rockets, but they never got around to fixing it:

//This is in the code for playerspawn (giving players their initial ammo at spawn):

//FIXME: extern this and check it when getting ammo from supplier, pickups or ammo stations!
if ( client->siegeClass != -1 &&
    (bgSiegeClasses[client->siegeClass].classflags & (1<<CFL_SINGLE_ROCKET)) )
        {
        client->ps.ammo[weaponData[m].ammoIndex] = 1;
        }

Because of this crappy unfinished code, mappers have avoided using CFL_SINGLE_ROCKET. But if it were fixed, it would give more possibilities for classes.

TheSil commented 9 years ago

I have updated the code a bit, now it is referencing single function for adding ammo. Although how i understood intent of CFL_SINGLE_ROCKET, it seems that you should be able to recharge that 1 rocket, but you should not have more than 1 rocket at one time.

deathsythe47 commented 9 years ago

Interesting, I interpreted it as you should only have 1 rocket ever, and you are unable to recharge it. Makes it like a disposable launcher.

Also this comment

//has only 1 rocket to use with rocketlauncher

doesn't say anything about a maximum ammo of 1, just says you have "only 1 rocket to use"

Maybe should be split into two classflags for both interpretations...

TheSil commented 9 years ago

That sentence is even worse, can be now interpreted that you just have 1 rocket to use with your rocketlauncher (standard rocket launcher i suppose), so if you happen to find another rocket, you could use that too.

Tbh, I think that even Raven didnt know exactly what should be expected. For now it seems to be consistent just treat it as a different limit (1 instead of 10). Having extra logic for checking if you have already used one rocket during your lifetime would be definitely interesting enhancement, but i wouldnt consider it as a part of this update.

deathsythe47 commented 9 years ago

Ok so I tested your commits quite a bit and there are a couple bugs:

If you have a class with CFL_EXTRA_AMMO, you can't be ammo'd up to double ammo again by tech healing or ammo canister. For example if I am demo with CFL_EXTRA_AMMO I get 20 mines. Allied tech/cans will only allow me a maximum of 10 mines. However, ammo dispenser still works fine, and will allow me to get 20 mines maximum.

Also, the notification "Obtained Ammo Dispenser" and the sound effect when you pick up a canister are missing now.

TheSil commented 9 years ago

Are you sure that you are trying latest commit for this issue? I have just tried it, just joined as a tech, dropped ammo on the floor, then switched to demo, dropped few mines so i have 18, then picked up the ammo and got to 20. I am also seeing Obtained Ammo Dispenser in this case.

deathsythe47 commented 9 years ago

I'll test again later

deathsythe47 commented 9 years ago

Ok, I tested it and while the ammo-receiving works fine, it no longer says "Obtained Ammo Canister" or plays the sound effect. Can we get those back?

deathsythe47 commented 8 years ago

Are you able to confirm "Obtained Ammo Canister" and sound effect?

TheSil commented 8 years ago

I have just tried it the same way i described before (dropping ammo as a tech, grabbing it as a demo), and i hear the sound and console says "Obtained Ammo Dispenser". Not sure what is different.