Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Vehicle turret magazine reload port #410

Open
wants to merge 9 commits into
base: unstable
Choose a base branch
from

Conversation

wersal454
Copy link

@wersal454 wersal454 commented Dec 6, 2024

What type of PR is this?

  1. Bug
  2. Change
  3. Enhancement
  4. Miscellaneous

What have you changed and why?

Information:

Ported + translated this mod/script https://github.com/ampersand38/reload-repack-turret-magazines , because it seems usefull

Please specify which Issue this PR Resolves (If Applicable).

"This PR closes #XXXX!"

Please verify the following.

  1. Have you loaded the mission in LAN host?
  2. Have you loaded the mission on a dedicated server?

Is further testing or are further changes required?

  1. No
  2. Yes (Please provide further detail below.)

How can the changes be tested?

Steps:


Notes:

@wersal454
Copy link
Author

couldn't find where to put a disable/enable parametr, probably could add it at the beginning of each sqf file

Copy link
Owner

@SilenceIsFatto SilenceIsFatto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally should make ReloadRepackTurretMagazines shorter, perhaps abbreviate it. Also change the class in CfgFunctions.hpp accordingly

@SilenceIsFatto SilenceIsFatto added the misc A miscellaneous change label Dec 27, 2024
Copy link
Owner

@SilenceIsFatto SilenceIsFatto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does this PR actually achieve? As far as I can tell there's no front end support to use the reload script, so isn't it a bit useless for 99% of people?

Comment on lines +809 to +815

class reload_repack_turret_magazines {
file = QPATHTOFOLDER(Scripts\RRTurretMagazines\scripts);
class postInit {};
class reloadTurret {};
class monitorMagazines {};
};
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't this class postInit overwrite the original A3A_fnc_postInit? That would be a very critical issue

If it's going to be called by execVM then you don't actually need to define it here anyway

private _ammoThisMag = selectMin [_fullCount, _totalAmmo];
_magsAdd pushBack [_currentMagazine, _turretPath, _ammoThisMag];
_totalAmmo = _totalAmmo - _ammoThisMag;
_message = _message + str _ammoThisMag + ([", ", ""] select (_i == _magsCount));
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[", ", ""]

Is this not an issue?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it works, so ...no? I mean, you tried this PR right?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried many other pulls and they "worked", until they didn't

Literally the point of reviewing, I see a syntax issue so I point it out?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

iirc, at first it was [",", ""], I added additional space after comma so created text would look better [", ", ""]

A3A/addons/scrt/Stringtable.xml Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
misc A miscellaneous change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants