Show Posts

This section allows you to view all posts made by this member. Note that you can only see posts made in areas you currently have access to.


Messages - karvanit

Pages: 1 2 3 [4] 5 6
46
Programming / Re: Questions about code / design decisions
« on: November 20, 2012, 03:03:53 am »
Doing N iterations is a lot cheaper than N comparisons. :P But yes I clearly stated that this isn't a huge deal, since the N amount of elements in a savegame yaml map is in the dozens at most.

When we just use node[val] for each of our values we do values searchs, each search is O(nodes). So we do O(N) * O(N) == O(N^2) actions.
When we iterate we do 'nodes' iterations * value comparisons, so still O(N) * O(N) == O(N^2) actions.
So they are exactly the same, but iteration is still the same cost if/when yaml-cpp changes to maps.

Just increasing my post count!  ;)

47
Programming / Refactoring translation
« on: November 20, 2012, 02:55:54 am »
I want to change the translation support but before I go through, I think it's better to ask for opinions and the general approval of the developers.
So, I am thinking of doing the following changes:
  • Create a Translator singleton. Translation is needed everywhere, no need pass an extra pointer around.
  • Allow parameters in the translated string  (see below for sample code). This will make translating whole sentences with information easier, and avoid concatenating parts in code (which is not correct form in many languages).
  • Allow translation of plural cases. This will require extra translation text in the language files.

Code comparison:
Code: (Simple case) [Select]
https:// Current code
obj->setText(_game->getLanguage()->getText("STR_SAMPLE"));

https:// New version
obj->setText(Translator::get().getText("STR_SAMPLE"));
Code: (Argument substitution) [Select]
https:// Current code
std::wsostream sstr;
std::wstring part1 = _game->getLanguage()->getText("STR_PART1");
std::wstring part2 = _game->getLanguage()->getText("STR_PART2");
sstr << part1 << someValue << part2 << someOtherValue;
obj->setText(sstr.str());

https:// New version
obj->setText(Translator::get().getText("STR_FULL_SENTENCE").arg(someValue).arg(someOtherValue));
https:// STR_FULL_SENTENCE contains %1 and %2 which get replaced.

Code: (Plural form) [Select]
https:// Current code
std::wsostream sstr;
std::wstring one = _game->getLanguage()->getText("STR_CASE_ONE");
std::wstring many = _game->getLanguage()->getText("STR_CASE_MANY");
if (someValue == 1)
  sstr << one;
else
  sstr << many;
sstr << someValue;
obj->setText(sstr.str());

https:// New version
obj->setText(Translator::get().getText("STR_SENTENCE", someValue));
https:// Language specific variants of STR_SENTENCE are searched.
https:// They contain %n which is replaced by someValue.

So, what do you think? Is this something you are willing to merge into OpenXcom?
PS: I will obviously handle all changes to existing code, and place proper strings in the English language files.
PS2: I can only guess at the proper argument placement for other languages though.

48
Programming / Re: Questions about code / design decisions
« on: November 20, 2012, 01:33:24 am »
Don't forget if we have smaller savegames, then the O(N) means much less. :)

But if you remove fields, you are always searching the entire list for nothing!  ;D

49
Programming / Re: Questions about code / design decisions
« on: November 20, 2012, 01:30:17 am »
That's what i thought.
And a conditional save() is also needed, checking if the value to be written is equal to the default or not. (and only write out if it's not the default)

For smaller save games yes. But not for compatibility. I don't think the savings will be enough to go to the trouble of doing it for every little field. If the size becomes a problem, it's much better to simply keep the files compressed.

50
Programming / Re: Questions about code / design decisions
« on: November 20, 2012, 01:26:26 am »
Regarding yaml-cpp deficiency:
Using the if-else and iterating in order is not significally faster, we still do O(N) comparisons for each node.
In addition, the code is harder to read and we'll get no benefit if/when yaml-cpp changes to keep values in maps instead of vectors. Using code like the following is my best solution so far:
Code: [Select]
const YAML::Node &node = doc["thingy"];
node["field1"] >> variable;
node["field2"] >> variable;
The code above only searches for "thingy" once, and the follonode["field1"] >> variable;wing searches are (hopefully) in the smaller contents.
And it is still easier to read and understand.

51
Programming / Re: Questions about code / design decisions
« on: November 20, 2012, 01:19:51 am »
The proper fix for default values is to have ALL fields initialized in a class' constructor with values representing the default, and perform conditional overwrite in [oad(). For some fields it may not really work, but it's the best possible solution.

Still, savegame compatibility right now IS a useless goal. The game is still in flux, saving a game should only be considered part of bug hunting (keep a save to have a test case for reproducing a bugs).

52
Work In Progress / Re: [Experimental] Modular rulesets
« on: November 18, 2012, 07:07:34 pm »
Most likely a bug, can you reproduce the problem reliably? What is your configuration? Can you provide a save that causes it? Are you using modified files? (ruleset, tiles, map)

53
Programming / Re: Questions about code / design decisions
« on: November 18, 2012, 03:55:12 am »
while i agree with you i also feel it is worth stating:

backwards compatibility of saved games should not be a concern in what is essentially a development version.

Post 1.0, yes, this should DEFINITELY be taken into account, but at this stage of development, new versions breaking old saved games is par for the course.

I'm 100% with warboy on this. Although saving only deviations from the default for the tiles (and even a little less verbose format for tile data) should be possible and desirable.

54
Programming / Re: Messing with yaml save games
« on: November 17, 2012, 01:43:57 pm »
This seems to be a Python file.

So save it as whatever.py and run it by
Code: [Select]
python whatever.py

55
Programming / Impementing: Alien missions
« on: November 17, 2012, 11:29:00 am »
I have now all the relevant data for the missions (thanks Volutar), and started moving them to ruleset format.
Another commit and I'll even start producing actual missions!

feature/alien-missions
.
Please provide feedback and comments / suggestions.

edit: Updated link.

56
Programming / Re: Questions about code / design decisions
« on: November 14, 2012, 12:05:21 am »
First of all, thank you for answering. I'll try to justify my positions and / or provide clarifications.
Don't YAML nodes just point to others? You'd still have to lookup the equivalent data structure in SavedGame, so I dunno about making the code clearer, although I guess the file format would be. But I don't know what yaml-cpp's support for them is like, there's no code examples.
I agree that this part of yaml-cpp is not clear. However, what it would allow us to do is to avoid the whole 'saveId()' part. We'll just have
Code: [Select]
out << YAML::Anchor(toAnchorText(object));
object->save(out);
. . .
out << YAML::Alias(toAnchorText(other->ref_object));
instead of
Code: [Select]
object->save(out);
. . .
other->ref_object->saveId();

True, it's longer, but I believe it's cleaner as we don't replicate YAML, we specificaly state what we do (emit a reference to an existing object). And we don't have to create the 'saveId()' part, it's as if the whole node is duplicated. It will also make it easier to abstract I/O away for object hierarchies.

Consistency I guess. Not every object is only loaded on construction (eg. modular rulesets), and it leaves the objects more open for alternate loading later.
I can understand the _more open_ part. But right now every class with I/O is either constructed and filled (via setters), or constructed and effectivelly overwritten (with 'load()'), except the ruleset, which is only partially overwritten in 'load()'. Which, I guess, is the reason for this API.
Singletons are... debatable. I don't really like them and I've always been taught not to like them. I don't see the need to arbitrarily restrict a class.
Singletons are, IMO, pretty-OOPified globals. But the idea of having globals is not bad in itself. In OpenXcom, we already use globals / singletons (RNG and Ufopaedia). The idea of a single GameEngine at least is not extreme.

In this case you just want it as a global so you don't have to pass around pointers, but I don't think that really fixes the problem. Most game entities shouldn't need or even care about the entire game state, they just need particular bits.
Most stuff needs the pointers only during I/O and creation time, but since eg Game is not global we have to keep it around for when we just need to pass it down to another class. And this tends to proliferate.

Whether it's better to have public fields or getter/setters is arguable. Personally I just do it for the consistency, if I do something once I do it all the time, and it makes fields easier to control and document (specially since OpenXcom is ever-changing). Yeah it becomes a lot of boilerplate code in some classes, but there's plenty of tools to generate it for you.
I agree, getters / setters are the best practice, since they allow you to change this internal representation. But due to the ad-hoc class APIs we are not reallly hiding anything. Still, keeping them allows us to eventually move to a better design.

Again, consistency. Probably has to do with old habits, I've always worked with pointers, so when references popped up I never got into them. What is their advantage over pointers? Aside from hiding the * which I don't really mind.
A reference states (in code) that what you want is always going to be there and will never change.
Code: [Select]
class Ufo
{
private:
  const RuleUfo &_rules;
};
This means that a Ufo requires a rules object and it will be the same for the lifetime of the Ufo object.
They also allow the beautiful (IMO) getter/setter functions:
Code: [Select]
class Example
{
private:
  int _field;
public:
  int field() const { return _field; } https://Getter
  int &field() { return _field; } https://Setter
};
Aren't rules read-only already? Anyways const-correctness is one of those things that takes forever to implement and forever to master. I don't know everywhere where it should go, though I've done my best to apply it across OpenXcom without breaking anything, so feel free to add more if you want.
Will do so, gradually, thanks for  the ok.

For example, take a Craft. You think "oh a craft has to move to places". So you need to change its position, just add a "setPosition". Then you think "but but but the craft should move to places, not be directly controlled". Ok you instead put a  "setDestination" (maybe you'd prefer calling it "moveTo", it's just semantics), it does what it says, you give it a destination and the Craft slowly goes there by its rules. Woo, OOP, etc. But wait what about when you need to move the craft instantly, like when it's transferred between bases? Whoops turns out you needed that direct position control after all! And you'll run into this kind of stuff all the time, and your wonderful design goes to pot as you need to get stuff done and players just want to play things.
I understand what you mean, but the specific example IS  flawed. A Craft does not "move" (fly in Geoscape) between bases, and the owning base is a matter of game database. So the destination has nothing to do with it. You need a 'setBase()' (which, in an ideal world, only transfers would use).

I'm not sure I understand your definitions here, so this is the way I see the OpenXcom design:

- The Game class handles the core engine stuff, the SDL subsystems, the state and event management. It also contains the game contents (resource, ruleset, save) out of convenience.

- The states take care of all the game behavior and interaction with the player, and are what connect and interact with everything else forming a running game.

- The SavedGame and related classes are intended to contain the variable game state, everything that needs to be saved to disk to preserve the game. The 'current game state of all objects' is the 'persistent data', so serializing Savegame saves the game and preserves the game state. This is ignoring superficial stuff which you might consider "state" like UI etc. Originally it was just intended to be "dumb" variable storage that the states would manipulate, but some behavior ended up leaking into it, so now it's kinda mixed between states and savegames.

- The Ruleset and related classes are intended to contain the fixed game variables. This is all the fixed definitions in the game rules, all the same fixed values from game to game. No behavior.

Hope that clears things up.

Yes, things are cleaner now. What I meant was that the 'keeper of rules' and 'keeper of persistent data' should be outside Game, in top level objects of their own. And that would make these three classes good candidates for singleton / global data. Since what they do is provide  a service to all the rest of the code, so instead of having a pointer to them in each class, we just keep a global pointer of them. Like we (effectivelly) do with RNG and Ufopaedia.

57
Programming / Re: [IMPORTANT] How to contribute code
« on: November 11, 2012, 08:53:36 pm »
Speaking only for myself, I consider this form of PR extremely pullable!

58
Programming / Re: Implemented: UFOs landing
« on: November 11, 2012, 02:10:13 pm »
- I assume you also implemented UFOs taking off again after a certain time (fixed time or variable set by mission-type or something?)?

- if they "fulfilled their mission" (aka landed and took off again after timer runs out), how does the "geoscape AI" take notice of this?

This does not add proper landing, it just contains the code that can make it possible and lands UFOs for some time before disappering them, to test the code.
 
- do they also take off again if assaulted on the ground but battle lost/aborted?
They get removed from the game. This is fine for now, but what does vanilla XCOM do?

59
Programming / Implemented: UFOs landing
« on: November 11, 2012, 01:55:50 pm »
On the ufo-landings branch I have code that makes UFOs land after reaching their waypoint. It should be bug-free, but since it changes the Ufo API slightly, I have posted here to get people to review it and comment.
The API changes:
  • Added getStatus() and setStatus() to Ufo class.
  • Changed *HoursCrashed() to *TimeOnGround().

The status is automtically changed when the UFO is damaged enough, or has it's altitude changed.
The status member is not saved, it is recalculated on load instead.

PS: I would prefer people using the GitHub comment-on-commit UI, but I'll also monitor this thread.


60
Programming / Re: [IMPORTANT] How to contribute code
« on: November 11, 2012, 05:13:10 am »
One feature in each PR, each PR in a separate branch, is the best way. Doing it like this, it's FAR easier for the integrator, and other people can easily help / comment / test on each feature.

Also, IMO, try to keep each commit clean and making a single (conceptual) change. eg HyperDecoder Ufopaedia, HyperDecode base facility, HyperDecoder UFO reporting = 3 commits. This makes it easier to comment / fix bugs, and to review the code changes.

Git tip: Try having a devel-feature local branch where you can do the actual work and mess with the history as often as you like (eg git rebase -i, git cherry-pick), and a clean feature branch you will push to GitHub.

Pages: 1 2 3 [4] 5 6