DUWS-R-Team / DUWS-R

ArmA 3 Dynamic Universal War System - Rebirth
42 stars 18 forks source link

add virtual arsenal support #66

Closed rlex closed 9 years ago

rlex commented 9 years ago

Issue

Fixes #67

Summary

Adds as "second" armory for those who prefer VA over VAS

Things to test

rlex commented 9 years ago

That's better? Also found bug on PlayerKilledEH, so it should now respawn both armories

rlex commented 9 years ago

Sorta. I use vim without any syntax highlighting. Feels like developing in stock notepad... Found sublime addon, should try it later.

rlex commented 9 years ago

Also there is crapload of trailing whitespaces in DUWS code. Like, thousands of them. Almost as annoying as mix of tabs and spaces

fritogotlayed commented 9 years ago

@rlex Yea, I'm with you on the whitespace. I've been trying to clean that up as I encounter it as well. No worries on developing in VIM. I'm using IntelliJ IDEA with a vim plugin so that completely makes sense to me. One of the things I love is that with IntelliJ IDEA I can have it print an arrow for tabs and 50%ish opacity dots for spaces. It really helps me catch those offenses before I commit them :-).

All in all everything in this PR looks really good except for the one question I had about the two for loops. Could you please educate me as to why you used two loops there? After that I'll go ahead and get this merged in.

fritogotlayed commented 9 years ago

Also, I really like your check boxes. I'm going to have to start using that at work. I didn't realize markdown let you do that and that git up picked that up in the summary for the PRs. That is way cool and thank you for showing it to me!

rlex commented 9 years ago

Which two loops, exactly? Not sure if i understood question.

fritogotlayed commented 9 years ago

It is the comment on source/dialog/request_support.sqf

rlex commented 9 years ago

Forgot to fix it, probably. Should look better now.

fritogotlayed commented 9 years ago

Just a heads up. I'll review this one after #59 is merged. It'll just make play testing quicker / easier in the future.

fritogotlayed commented 9 years ago

@rlex I just checked out and built this branch. When I launch the mission the initial screen to place the HQ is not shown. I rebased up to master and it still didn't work. Checking out master without the code here had everything working.

My master HEAD is at ddd7f4e if that helps. That issue will need to be resolved before I merge into upstream.

fritogotlayed commented 9 years ago

@rlex Do you think you'll have time to debug what is causing the initial screen not to show up? I think I have found the issue and can make a fix but I can't push the update to your branch. I found the issue in the INIT script by re-formatting the code to make it easier to read. If you don't think you'll have time to research it I can close this PR and create a new one with the fix. I just figured you might want credit for the work you've done here.

fritogotlayed commented 9 years ago

Sorry. That was kinda dickish. I got distracted. There was a missing }; on the first block of code.

rlex commented 9 years ago

so i'm basically not closing if (support_armory_available) then { block?

fritogotlayed commented 9 years ago

Right.

if (support_armory_available) then {
    hq_blu1 addaction ["<t color='#ff0066'>Armory 1 (VAS)</t>","VAS\open.sqf", "", 0, true, true, "", "_this == player"];
    hq_blu1 addaction ["<t color='#ff0066'>Armory 2 (VA)</t>","bisArsenal.sqf", "", 0, true, true, "", "_this == player"];
    {
        _x addaction ["<t color='#ff0066'>Armory 1 (VAS)</t>","VAS\open.sqf", "", 0, true, true, "", "_this == player"];
        _x addaction ["<t color='#ff0066'>Armory 2 (VA)</t>","bisArsenal.sqf", "", 0, true, true, "", "_this == player"];
    } forEach (Array_of_FOBS);
}; // <-- This is missing

That is the big reason I dislike things like blah;};. It can get hard to read and keep everything straight.

I much prefer things like this...

if (foo)
{
    // blah code
}

or

if (foo) {
    // blah code
};
fritogotlayed commented 9 years ago

@rlex Alright, this is probably a dumb question but I'm going to ask anyways. How do I get additional ammo with the new BIS armory?

rlex commented 9 years ago

clicking suit / vest / backpack will allow you to grab more ammo / rockets / explosives / etc in their menu

fritogotlayed commented 9 years ago

I herp-derped this regarding getting more ammo. This seems to work at the HQ and respawn. Merging now and will have a follow up commit to fix the formatting in a couple of spots.