Forum index VCMI Project - Heroes 3: WoG recreated
Forum of the project aiming to recreate best turn-based strategy ever!

FAQFAQ  SearchSearch  MemberlistMemberlist  UsergroupsUsergroups  StatisticsStatistics
RegisterRegister  Log inLog in  AlbumAlbum  DownloadDownload

Previous topic :: Next topic
Trunk discussion / complains
Author Message
Warmonger 
VCMI programmer


Age: 28
Joined: 07 Jun 2008
Posts: 1541
Location: Warsaw, Poland
Posted: 2013-03-03, 19:26   Trunk discussion / complains

I think we should have a seprate thread about upcoming or recent trunk changes that may need some discussion or clarification. Especially when they are not related to any particular hot topic, such as modding system.

So, my first complain is about recent commit from Ivan:
r3257

While level 9 Dragons are resonable, they are not standard and certainly not disscussed before.

Also, what does 'special" for creature mean? I remember that special heroes do not show up in tavern, but neutral creatures should spawn as well as others. Certainly original RMG is able to spawn them, unlike campaign heroes for example.
_________________
Think twice if you really need to send me private message. Use public forum for general questions.
DJ Warmonger blog
beegee wrote:
Warmonger, you are the best!
 
 
     
AVS 

Joined: 25 Feb 2011
Posts: 554
Location: Russia
Posted: 2013-03-03, 20:04   Re: Trunk discussion / complains

Warmonger wrote:

While level 9 Dragons are resonable, they are not standard and certainly not disscussed before.


Same question about "yours" level 10 dracolich in rev 3254
 
     
Ivan 
VCMI programmer

Age: 25
Joined: 08 May 2009
Posts: 1438
Location: Ukraine
Posted: 2013-03-03, 20:23   

Quote:
While level 9 Dragons are resonable, they are not standard and certainly not disscussed before.

My bad. For some reason I thought that Crystal\Rust are lvl 9
Interesting fact is that internally in H3 all dragons have level 7 - not even close to 10

Quote:
Certainly original RMG is able to spawn them, unlike campaign heroes for example.

Now that's interesting. Reason for this change was due to fact that these 6 creatures (dragons + enchanters and snipers) can't be recruited in Refugee Camp and H3 also does not spawns them as random creatures (despite that it will spawn other neutrals). RMG does not spawns their dwellings either - only creatures themselves.
 
     
AVS 

Joined: 25 Feb 2011
Posts: 554
Location: Russia
Posted: 2013-03-03, 20:58   

So, 2 questions
1) should level 9+ creatures be legal in VCMI
2) which creatures should be strictly special (ammoCart, fisrtAidTent, catapult and "gods" certainly should but I`m not sure about about commanders and even ballista)
 
     
Warmonger 
VCMI programmer


Age: 28
Joined: 07 Jun 2008
Posts: 1541
Location: Warsaw, Poland
Posted: 2013-03-03, 21:03   

Quote:
RMG does not spawns their dwellings either - only creatures themselves

Neutral dwellings DO spawn in RMG if the terrain is set to neutral. Clash of Dragons template is based on that trick.

Clash of Dragons.rar
Download 735 Time(s) 128.12 KB

_________________
Think twice if you really need to send me private message. Use public forum for general questions.
DJ Warmonger blog
beegee wrote:
Warmonger, you are the best!
 
 
     
Ivan 
VCMI programmer

Age: 25
Joined: 08 May 2009
Posts: 1438
Location: Ukraine
Posted: 2013-03-03, 21:28   

AVS,
1) They are legal in terms that they can be handled by engine. But for example I just found one bug related to such levels: Clone spell won't work on dragons. This is not how it works in H3.

2) Those that are normally not available in-game - these objects can be placed manually on map or used in config but they can't be subject to any random picking and by default - disabled on map. So both commanders and ballistas are "special"

Warmonger, I'm wondering - is this a bug in H3 or a feature?

But in any event I think we shouldn't add different behavior between RMG and game randomization. So we should either enable these creatures everywhere or disable them instead of something inbetween as in H3.
 
     
Warmonger 
VCMI programmer


Age: 28
Joined: 07 Jun 2008
Posts: 1541
Location: Warsaw, Poland
Posted: 2013-03-04, 19:33   

A package with new icons for creature window from Kuririn.

BonusIcons.rar
Download 753 Time(s) 26.97 KB

_________________
Think twice if you really need to send me private message. Use public forum for general questions.
DJ Warmonger blog
beegee wrote:
Warmonger, you are the best!
 
 
     
AVS 

Joined: 25 Feb 2011
Posts: 554
Location: Russia
Posted: 2013-03-04, 21:30   

I`m currently working on configurable bonus icons.

UPD: committed.
 
     
Tow dragon 
VCMI Programmer


Joined: 01 Feb 2008
Posts: 1004
Location: Kraków
Posted: 2013-03-14, 23:23   

What are those 'Mantis integration' commits for? I'm just curious.
_________________
:)
 
     
AVS 

Joined: 25 Feb 2011
Posts: 554
Location: Russia
Posted: 2013-03-15, 09:01   

Tow dragon wrote:
What are those 'Mantis integration' commits for? I'm just curious.

Now svn log (in tortoisesvn at least) shows bug ids and links to bugtracker.
 
     
Tow dragon 
VCMI Programmer


Joined: 01 Feb 2008
Posts: 1004
Location: Kraków
Posted: 2013-03-15, 17:01   

Quote:
Now svn log (in tortoisesvn at least) shows bug ids and links to bugtracker.


Nice! Good idea.
_________________
:)
 
     
beegee 
VCMI programmer

Joined: 12 Dec 2009
Posts: 247
Location: Stuttgart, Germany
Posted: 2013-04-05, 12:53   

I would like to rename my added sub-folders in /lib and /client to lowercase e.g. RMG to rmg and Logging to logging. It seems to be that lowercase named folders are more often used and consistency in naming folders and files is nice. I want to do this in a separate SVN commit. All references in project files, cmake, etc.. will be updated. If it's not ok, please tell it:)
 
     
Warmonger 
VCMI programmer


Age: 28
Joined: 07 Jun 2008
Posts: 1541
Location: Warsaw, Poland
Posted: 2013-04-05, 14:25   

I think RMG should stay as it is, as it's acronym. No need to impose strange naming conversions on files which nobody ever needs to browse.

What worries me more is a rapidly groing number of files that are almost empty. For example CLogFileTarget.cpp has less than 30 lines and new multiple headers are nothing but trivial comments. Constructor, destructor... what's the use? Everyone can tell the arguments and returned type from function signature and the comments do not add any valuable info to it.
_________________
Think twice if you really need to send me private message. Use public forum for general questions.
DJ Warmonger blog
beegee wrote:
Warmonger, you are the best!
 
 
     
beegee 
VCMI programmer

Joined: 12 Dec 2009
Posts: 247
Location: Stuttgart, Germany
Posted: 2013-04-05, 19:32   

Warmonger wrote:
I think RMG should stay as it is, as it's acronym.

It doesn't matter if the name is acronym in my opinion. Simply write everything in lowercase and camel-case to separate words. For example CVCMIServer could be written as CVcmiServer or VCMIDirs as VcmiDirs which is more readable. But all in all it has a lot to do with personal preference. I think that the first letter of folder names should simply be lowercase.

Warmonger wrote:
What worries me more is a rapidly groing number of files that are almost empty. For example CLogFileTarget.cpp has less than 30 lines

Low count of source code lines in h/cpp files isn't a bad sign as well as high count of source code lines isn't automatically a good sign. The logging API doesn't consist of 1 or 2 classes, it is not that trivial. I think different parts of the API are clearly separated and loosely coupled. Would it be that much better to create a file LogTargets / LogTargetClasses / ILogTarget and put all target classes in one compilation unit or to put all logging classes in one file named CLogger.h/cpp? I don't think so.

Warmonger wrote:
Everyone can tell the arguments and returned type from function signature and the comments do not add any valuable info to it.

The comments are just for the sake of completeness. Simply skip over it, it they don't provide any valueable information:)
 
     
Ivan 
VCMI programmer

Age: 25
Joined: 08 May 2009
Posts: 1438
Location: Ukraine
Posted: 2013-04-05, 21:02   

I don't think that multi-line comments are needed for trivial functions. For example this:
Code:
/**
  * Sets the color mapping.
  *
  * @param colorMapping The color mapping.
  */
void setColorMapping(const CColorMapping & colorMapping);

Your description duplicates function name. And description of parameter duplicates parameter definition.
Thus I can get same amount of information by reading one line: function definition instead of 6 lines. And considering that vertical screen on monitor is limited in this case I need to spend more time on reading than I will do when this comment is not present.
 
     
Display posts from previous:   
Reply to topic
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 vote in polls in this forum
You cannot attach files in this forum
You can download files in this forum
Add this topic to your bookmarks
Printable version

Jump to:  

Powered by phpBB modified by Przemo © 2003 phpBB Group

Hosting provided by DigitalOcean
Page generated in 0.05 second. SQL queries: 14