ToME: the Tales of Maj'Eyal

Everything about ToME
It is currently Mon Jun 26, 2017 12:12 am

All times are UTC




Post new topic Reply to topic  [ 30 posts ]  Go to page Previous  1, 2
Author Message
PostPosted: Sat Dec 19, 2015 8:50 pm 
Offline
Spiderkin

Joined: Sat Mar 18, 2006 12:48 pm
Posts: 480
Lord Estraven wrote:
Okay, so, the traps code is not only entangled with Rogues' monster traps, but also with searching, hidden doors, and chest traps.

I am considering removing all of them.
- Searching and hidden doors are pretty useless, especially without traps, and don't add much flavor
- Chest traps are very, very heavily dependent on floor trap code AFAICT

I will try to preserve them for now though.

(But yes, it would be very nice indeed to yank out all that code!)

Edit: I'll keep trapped chests, hidden doors, and searching for now. Monster traps will be gone though.

Edit: or not, because t_info includes chest traps too. BLARGH.


Yes -- welcome to the world of the traps code :). It's all very tangled and messy.

I think maybe we could go in steps here... perhaps removal of the various non-Rogue trap effects would be a decent first step? (I.e. removing the "swap items" and "drop everything" effects, and such.)


Top
 Profile  
 
PostPosted: Sat Dec 19, 2015 9:06 pm 
Offline
Uruivellas

Joined: Tue Dec 13, 2005 12:35 am
Posts: 702
FWIW I have it compiling now - just a few link errors. I should be able to commit something pretty shortly.

Searching and hidden doors will still be there (albeit stupid and useless) for now. Chest traps will be gone.

Edit: alright, it builds and links. Time to see if it runs.

Edit: ooh nice, there's a buffer overflow. Thank you libasan! Gotta fix that first...

Edit: oh dear. r_info_flags6 needs a placeholder. It's size-dependent. YAY.

Edit: dependent on index too by the looks of it. WTH? :(

Edit: in any case, there we go, revision #1 compiles and runs without crashing. YAY. Will commit and push to the new branch in a minute.

_________________
"These aren't the hobbits you're looking for."


Top
 Profile  
 
PostPosted: Sat Dec 19, 2015 9:25 pm 
Offline
Uruivellas

Joined: Tue Dec 13, 2005 12:35 am
Posts: 702
Alright. Compiles, links, and plays. Pushed to Gitlab and merge request filed.

_________________
"These aren't the hobbits you're looking for."


Top
 Profile  
 
PostPosted: Sat Dec 19, 2015 9:37 pm 
Offline
Uruivellas

Joined: Tue Dec 13, 2005 12:35 am
Posts: 702
Whoops, I just realized I forgot to get rid of Disarming skill. And Sneakiness. And Create Traps.

I'll leave that to my future updates I guess...

_________________
"These aren't the hobbits you're looking for."


Top
 Profile  
 
PostPosted: Sun Dec 20, 2015 12:31 pm 
Offline
Uruivellas

Joined: Tue Dec 13, 2005 12:35 am
Posts: 702
On further inspection, something is very wrong here - some kind of index error I think. Items' intrinsic to-hit bonuses seem to be getting turned into AC bonuses. :( Locating this will probably be hard, but I'll take a look.

Edit: yeah, everything's off by one I think? To-hit is getting turned into to-dam, and to-dam is getting turned into AC. Sigh.

_________________
"These aren't the hobbits you're looking for."


Top
 Profile  
 
PostPosted: Sat Dec 26, 2015 4:56 pm 
Offline
Uruivellas

Joined: Tue Dec 13, 2005 12:35 am
Posts: 702
Okay - I'm belatedly getting back to this. GDB should let me figure out where things are going wrong, if I can just guess which function is involved. Hopefully I'll have an updated push request by the end of today.

Edit: looks like "good" weapons are getting a_am_aux_2 (for armor) instead of a_m_aux_1 (for weapons). Not sure why yet.

_________________
"These aren't the hobbits you're looking for."


Top
 Profile  
 
PostPosted: Sat Dec 26, 2015 5:21 pm 
Offline
Uruivellas

Joined: Tue Dec 13, 2005 12:35 am
Posts: 702
... I don't think I introduced this bug, actually. In fact, I suspect it is very old indeed, and was just being masked by some other bug.

Code:
       case TV_HAFTED:
        case TV_POLEARM:
        case TV_MSTAFF:
        case TV_SWORD:
        case TV_AXE:
        case TV_BOOMERANG:
        case TV_BOW:
        case TV_SHOT:
        case TV_ARROW:
        case TV_BOLT:
        case TV_DAEMON_BOOK:
                {
                        /* UGLY, but needed, depending of sval teh demon stuff are eitehr weapon or armor */
                        if (o_ptr->sval == SV_DEMONBLADE)
                        {
                                if (power) a_m_aux_1(o_ptr, lev, power);
                        }
                        else
                        {
                                if (power) a_m_aux_2(o_ptr, lev, power);
                        }
                        break;
                }


Those are reversed! The first should be a_m_aux_2, the second a_m_aux_1.

(And this, of course, is why functions should have descriptive names. :( )

Edit: yup, reversing those works. I'll create another branch and another pull request.

Edit 2: the above is in [b]object2.cc[/i] FYI.

_________________
"These aren't the hobbits you're looking for."


Last edited by Lord Estraven on Sat Dec 26, 2015 5:29 pm, edited 1 time in total.

Top
 Profile  
 
PostPosted: Sat Dec 26, 2015 5:29 pm 
Offline
Uruivellas

Joined: Tue Dec 13, 2005 12:35 am
Posts: 702
@AnonymousHero

I actually can't file a pull request at the moment, because the GitLab page for doing so is 404 (!). In any case this is any easy fix, I figure you can make the change when you next have time.

_________________
"These aren't the hobbits you're looking for."


Top
 Profile  
 
PostPosted: Sun Dec 27, 2015 7:05 pm 
Offline
Spiderkin

Joined: Sat Mar 18, 2006 12:48 pm
Posts: 480
Lord Estraven wrote:
Code:
       case TV_HAFTED:
        case TV_POLEARM:
        case TV_MSTAFF:
        case TV_SWORD:
        case TV_AXE:
        case TV_BOOMERANG:
        case TV_BOW:
        case TV_SHOT:
        case TV_ARROW:
        case TV_BOLT:
        case TV_DAEMON_BOOK:
                {
                        /* UGLY, but needed, depending of sval teh demon stuff are eitehr weapon or armor */
                        if (o_ptr->sval == SV_DEMONBLADE)
                        {
                                if (power) a_m_aux_1(o_ptr, lev, power);
                        }
                        else
                        {
                                if (power) a_m_aux_2(o_ptr, lev, power);
                        }
                        break;
                }


Those are reversed! The first should be a_m_aux_2, the second a_m_aux_1.


It actually even worse than that: The if-check completely ignores the TVAL, and so an object with any of the above TVALs and which happens to have SVAL == SV_DEMONBLADE gets treated with the "workaround". The quality of the T2 code is very, very dubious overall.

If you can show a quick way to reproduce, I'd be happy to fix this.


Top
 Profile  
 
PostPosted: Sun Dec 27, 2015 7:32 pm 
Offline
Uruivellas

Joined: Tue Dec 13, 2005 12:35 am
Posts: 702
Blargh! Sorry for the oversight there.

To reproduce, just checkout and compile my 'v2.4-trap-removal' branch:
https://gitlab.com/miramor/tome2/tree/v2.4-trap-removal

For some reason, "good" weapons in that branch reliably get AC bonus (rather than to-hit/to-dam) unless the above is fixed.

_________________
"These aren't the hobbits you're looking for."


Top
 Profile  
 
PostPosted: Sun Dec 27, 2015 9:35 pm 
Offline
Spiderkin

Joined: Sat Mar 18, 2006 12:48 pm
Posts: 480
Hang on... that's a bug you've introduced yourself. :) (EDIT: ... which kind of makes sense. I was having a hard time imagining how such an obvious bug should creep through.)

The original code in v2.4 reads:

Code:
   ...
   case TV_SHOT:
   case TV_ARROW:
   case TV_BOLT:
   case TV_TRAPKIT:
      {
         if (power) a_m_aux_1(o_ptr, lev, power);
         break;
      }

   case TV_DAEMON_BOOK:
      {
         /* UGLY, but needed, depending of sval teh demon stuff are eitehr weapon or armor */
         if (o_ptr->sval == SV_DEMONBLADE)
         {
            if (power) a_m_aux_1(o_ptr, lev, power);
         }
         else
         {
            if (power) a_m_aux_2(o_ptr, lev, power);
         }
         break;
      }


IOW, you've removed too much code. Fall-through results in the bug.

Btw, not sure why you should be having problems with Qt Creator's (3.5 and 3.6) Find Usages... it works almost flawlessly for me. It does seem to sometimes have a little trouble with macros (#define'd values, mostly), but you can Ctrl-click through to the macro definition and use Find Usages on that instead to get it working.


Top
 Profile  
 
PostPosted: Sun Dec 27, 2015 10:15 pm 
Offline
Uruivellas

Joined: Tue Dec 13, 2005 12:35 am
Posts: 702
D'oh! That's what I get for deleting stuff on automatic. :oops: Thanks @AnonymousHero.

_________________
"These aren't the hobbits you're looking for."


Top
 Profile  
 
PostPosted: Mon Dec 28, 2015 8:21 am 
Offline
Spiderkin

Joined: Sat Mar 18, 2006 12:48 pm
Posts: 480
I've left a couple of comments on the gitlab PR page, but don't bother uploading a new version of the PR just yet.

I think I've just about decided to move the repository to GitHub sometime soon (next day or so, at a guess). Unfortunately, it appears that GitHub is basically the only viable option right now. (Too many unexplained weird issues and generally incredible slowness of GitLab means that it's annoying to use.)

I'll see if it's feasible to at auto-mirror it to Gitlab or something, but development will take place on GH once the move it complete. I'll announce in a new thread + the maintenance thread once it's done.

EDIT: Moved, but there'll be no mirroring. (See the other post for details.)

Feel free to submit a new PR against the new repository. You can should be able to do copy all your changes across by creating a repository on GH, adding that as a remote in your current checkout ("git remote add github git@github.com:/....") and pushing your branch to that remote.


Top
 Profile  
 
PostPosted: Sun Feb 14, 2016 5:25 pm 
Offline
Uruivellas

Joined: Tue Dec 13, 2005 12:35 am
Posts: 702
So, an update: I managed to ditch large portions of trap-related code in ToME, on one of my branches. One thing I neglected though, was that Theme still had all kinds of trap stuff. This proved difficult to get rid of.

That was several weeks ago... I'm still thinking about how this stuff might be handled decently. I don't think it can be done using skills, at least not in a balanced or elegant way.

Best way might be something stupidly simple. Like, for instance, making levitation/flying confer total immunity to traps. That is inconsistent vs. monster traps, but whatever; I'll test it out in a branch I guess.

_________________
"These aren't the hobbits you're looking for."


Top
 Profile  
 
PostPosted: Sun Mar 13, 2016 10:34 pm 
Offline
Uruivellas

Joined: Tue Dec 13, 2005 12:35 am
Posts: 702
Okay, trap removal take #3. Yanked all the trap code; got rid of all the item-related crashes I could find; and rearranged the skill tree to make sense again.

The (currently) final result is in the rm-traps branch:

https://github.com/miramor/tome2/tree/rm-traps

Hope that is sufficient for now. :mrgreen:

_________________
"These aren't the hobbits you're looking for."


Top
 Profile  
 
Display posts from previous:  Sort by  
Post new topic Reply to topic  [ 30 posts ]  Go to page Previous  1, 2

All times are UTC


Who is online

Users browsing this forum: No registered users and 1 guest


You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot post attachments in this forum

Search for:
Jump to:  
Powered by phpBB® Forum Software © phpBB Group