Ticket #1222 (closed enhancement: fixed)

Opened 14 months ago

Last modified 13 months ago

[PATCH] Add Option to Disable Particles

Reported by: k776 Owned by: rogue-spectre
Priority: Nice to Have Milestone: Alpha 10
Component: Core engine Keywords: patch
Cc:

Description

Either in game (dev overlay) or in local.cfg as a renderer.particles = false option, allow people to disable particles, because they do take processing power and some computers can't handle it.

Attachments

disable_particle.patch (5.1 KB) - added by rogue-spectre 13 months ago.
patch to disable particles
disable_particle_correction.patch (3.2 KB) - added by rogue-spectre 13 months ago.
correction
disable_particle_correction.2.patch (3.6 KB) - added by rogue-spectre 13 months ago.
disable_particles_by_config_file.patch (3.5 KB) - added by rogue-spectre 13 months ago.
disable_particle_file_and_gui.patch (6.4 KB) - added by rogue-spectre 13 months ago.
disable_particle_file_and_gui.2.patch (6.4 KB) - added by rogue-spectre 13 months ago.

Change History

comment:1 Changed 13 months ago by historic_bruno

  • Summary changed from Disable Particles to Add Option to Disable Particles

comment:2 Changed 13 months ago by rogue-spectre

Before i lose or delete what i've made(i'm beginner with svn), one can find here a way to disable particles:

'ps/GameSetup/config.h

    extern bool g_RenderParticles;

config.cpp

    bool g_RenderParticles=false;

in static void LoadGlobals?()

	CFG_GET_USER_VAL("renderer.particles", Bool, g_RenderParticles);

renderer/Renderer.h ~l119 in Struct Options

    bool m_RenderParticles

~l82 in enum Option

    OPT_RENDERPARTICLES

Render.cpp ~647 in setOptionBool

case OPT_RENDERPARTICLES
         m_Options.m_RenderParticles=value;
         break;

in getOptionbool

 case OPT_RENDERPARTICLES
         return m_Options.m_RenderParticles;

~l 1436

     if(m_Options.m_RenderParticles)
     {
         RenderParticles();
     }

binaries/data/config/default.cfg

     renderer.particles = true
Last edited 13 months ago by rogue-spectre (previous) (diff)

comment:3 Changed 13 months ago by k776

  • Keywords review added

Hey. If you could make that into a patch based on the latest SVN, that would be good.

comment:4 Changed 13 months ago by rogue-spectre

Yes, i try to get svn working… i'm just a beginner. I go back to the documentation to see what i've missed in the svn configuration.

Last edited 13 months ago by rogue-spectre (previous) (diff)

Changed 13 months ago by rogue-spectre

patch to disable particles

comment:5 Changed 13 months ago by rogue-spectre

I must change one thing in the patch…

Changed 13 months ago by rogue-spectre

correction

comment:6 Changed 13 months ago by rogue-spectre

I will try to add an option to the gui in order to disable particle by this way.

Changed 13 months ago by rogue-spectre

comment:7 Changed 13 months ago by rogue-spectre

The third one is the "good", i hope (i had forgoten CFG_GET_USERVAL in config.cpp )

comment:8 Changed 13 months ago by k776

  • Keywords patch added

comment:9 Changed 13 months ago by historic_bruno

I think the new option should be named particles or renderparticles to match the other render config options, they don't use the renderer prefix.

comment:10 Changed 13 months ago by rogue-spectre

You suggest that i change only "renderer.particles" to "renderparticles" or "particles" or that i change all the variables i've introduced in order to suppress the "render" prefix ? The reason i chose "renderer.particles" was just because it was suggested in the description of the ticket, but it can be changed.

comment:11 Changed 13 months ago by k776

  • Summary changed from Add Option to Disable Particles to [PATCH] Add Option to Disable Particles

Hey rogue-spectre. Sorry for the confusion. When I wrote the original ticket, I just added a config example, not the actual config value wanted. My bad! :-(

So historic_bruno means changing the setting in default.cfg from renderer.particles = false to simply particles = false (which will also mean changing CFG_GET_USER_VAL("renderer.particles", Bool, g_RenderParticles); to CFG_GET_USER_VAL("particles", Bool, g_RenderParticles);

By the way, particles should be true by default, not false.

comment:12 Changed 13 months ago by k776

That said, "g_FancyWater" isn't "g_RenderFancyWater" so "g_RenderParticles" probably should drop the "Render" and become "g_Particles". So I think for consistency and if not much trouble, drop the "Render" part altogether.

I'm quite keen to get this into Alpha 10 because my Mac is super slow and the less it has to do, the better.

comment:13 Changed 13 months ago by rogue-spectre

Ok, i will remove the prefix "render" in all the variables i've introduced… by the way if i can get the option disable particle by the gui, can i put in into the patch, or must that be done in an other patch ?

Changed 13 months ago by rogue-spectre

comment:14 Changed 13 months ago by rogue-spectre

The patch with "render" prefix removed.

Changed 13 months ago by rogue-spectre

Changed 13 months ago by rogue-spectre

comment:15 Changed 13 months ago by rogue-spectre

  • Owner set to rogue-spectre

and an other with disable particles in the setting box… hope it will be good for you. Time to sleep for me ;)

comment:16 Changed 13 months ago by vts

Hi, thanks for taking the time to create a patch for this :)

In general, the patch seems to work great! Some minor comments:

  • In Renderer.cpp, it looks like the ogl_WarnIfError() call can be moved into the if-statement.
  • CRenderer's constructor sets some defaults for its various options, which are then later overwritten by the values read from the config files. Your patch only seems to be doing the latter, albeit right after the constructor. For robustness though, it's a good idea to set a default (conservative) value in the constructor as well.

For completeness, here are some stylistic guidelines we'd like to mention. Please don't misinterpret; not trying to criticize, merely trying to help you in the process of writing patches :)

  • In session.xml, the indentation looks a bit off. We try to maintain consistent indentation in all our files, so the general rule of thumb is: use the same indentation style that's already there. (In this case, it looks like tabs).

I'll commit your patch soon, and thank you!

comment:17 Changed 13 months ago by vts

  • Status changed from new to closed
  • Resolution set to fixed

In 11658:

Add option to disable particles, based on patch by rogue-spectre. Fixes #1222.

comment:18 Changed 13 months ago by k776

  • Keywords review removed

@rogue-spectre Thank you for your work on this. It works great. Please feel free to take up another ticket and have a go at implementing a patch for it. We're always looking out for new regular contributors to give us a hand with the game.

Note: See TracTickets for help on using tickets.