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

Comments in the code? #81

Closed
ajes opened this issue Jun 26, 2018 · 6 comments
Closed

Comments in the code? #81

ajes opened this issue Jun 26, 2018 · 6 comments

Comments

@ajes
Copy link
Contributor

ajes commented Jun 26, 2018

Should we add comments to the source code?
Eg:

diff --git a/Source/stores.cpp b/Source/stores.cpp
index 7c43d8b..04d0265 100644
--- a/Source/stores.cpp
+++ b/Source/stores.cpp
@@ -1438,15 +1438,15 @@ void __cdecl S_StartWSell()
 // 6A09E4: using guessed type int stextsmax;
 // 6A6BB8: using guessed type int stextscrl;
 
+// Check if item should be recharged^M
 bool __fastcall WitchRechargeOk(int i)
 {
-       bool rv; // al
-
+       bool rv; // return bool value. 0 - no recharge available. 1 - recharge available^M
        rv = 0;
-       if ( plr[myplr].InvList[i]._itype == ITYPE_STAFF
-         && plr[myplr].InvList[i]._iCharges != plr[myplr].InvList[i]._iMaxCharges )
+       if ( plr[myplr].InvList[i]._itype == ITYPE_STAFF // is item a STAFF?^M
+         && plr[myplr].InvList[i]._iCharges != plr[myplr].InvList[i]._iMaxCharges ) // does STAFF require recharge?^M
        {
-               rv = 1;
+               rv = 1; // require recharge^M
        }
        return rv;
 }
@@ -1515,6 +1515,7 @@ void __cdecl S_StartWRecharge()
                do
                {
                        //_LOBYTE(v3) = WitchRechargeOk(inv_num);
+                       // if item require recharge, add it to the list.^M
                        if ( WitchRechargeOk(inv_num) )
                        {
                                v8 = 1;

@sunverwerth
Copy link
Contributor

Absolutely.

@ghost
Copy link

ghost commented Jun 26, 2018

Eventually, but not a priority. I've tried to only add comments where there are decompiler errors, to help make them easier to spot. I'd prefer to keep it this way for awhile so self-explanatory functions don't get clogged. E.g.

if( leveltype == LT_CAVES ) // if the level type is in the caves
{
    if( player == myplr ) // if the current player is my player
    {
        CastSpell(SPELL_TOWNPORTAL); // cast town portal
        PlaySFX(SOUND_PORTAL); // play town portal sound

@MonkeyWeiti
Copy link

As this is C++ and not C# I would still stick to Dennis Doomens Guidlines for commenting code AV2310 & AV2316

https://csharpcodingguidelines.com/documentation-guidelines/

@ghost
Copy link

ghost commented Jul 8, 2018

I would like to see comments on address spaces in the original exe . this will make it easier for me when I say need to look at the original function in asm.

@mewmew
Copy link
Contributor

mewmew commented Jul 12, 2018

I would like to see comments on address spaces in the original exe . this will make it easier for me when I say need to look at the original function in asm.

This was part of the original source (as taken from the decompiler), but was removed in rev 49a6f4f and superseded by Support/surgery.xls.

I wouldn't mind if these addresses were added again to the source code to facilitate reversing. Later when we hit 1.0 they can be removed. What do you think @galaxyhaxz?

@ghost
Copy link

ghost commented Aug 10, 2018

Yeah it might be a good idea to add those back at least to the .h files. We'll add comments later on as everything is gradually cleaned up.

@ghost ghost closed this as completed Aug 10, 2018
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants